Hi Markus, 

Since you are the only one who replied, I assume, you are the person that is 
maintaining Admins jQuery? Will I be sending you the patch?

If you agree with me, I'll add a bug "code prone to bugs". 



Let me start from the beginning - to explain why it is important, and why I am 
not going to update code in tabs other that oc.upload.new.js - except for 
leaving a comment, that the approach if obsolete, and to follow the approach we 
will pick.
I assume it the one from 
>> oc.admin.js <<  --> Approach 2


====================================

I told one of our engineers to develop a new tab for CA monitoring. What he did 
is:
 1. he looked ofer oc.*.js files, and noticed that there were different 
approaches.
 2. he picked one that looked like the "latest" - there were couple of reasons 
he picked "oc.upload.js".
 3. he developed new tab based upon "oc.upload.js".
 4. he tested it - it worked. 
 5. he committed his code for me to approve it.
 
=========
I took it over, and noticed that it works great, only that now, when I try to 
upload new file to the server, a list of failed upload opens. <I would probably 
missed it if this would be tab statistics. not file upload>
<<<<<<<<<<
oc.upload.new.js --> this.init = 
oc.camanager.new.js --> this.init = 
<<<<<<<<<<


I cannot blame my engineer he did a great job, he just do not have enough 
experience with JavaScript yet, and he did that in no time, I proposed him a 
solution, we have it running.





Markus, 
you mention:
=================
> I've also ran into that issue. I solved this by
> giving the init 
> function another name.

You didn't solved the issue. You walked by it, and I do not mind, since you 
were focusing on adding new functionality - but adding new functionality would 
be going smoothly if you would not have to deal with things like this. Other 
that this, what was easy for you, may be a roadblock for somebody else - what 
if you would change a function name to something that already exists somewhere 
else? What if your code would be working properly,  but the fix would ruin 
something else? 

This is what I want to deal with different approaches.


> But
> then you 
> should think about cleaning all of the files.

Well... I have knowledge and resources to fix oc.upload.new.js, and leave a 
comment in most of other files, that if somebody will be working with the 
files, she/he must not follow approach from this file.

Next think what I would do, is give this task a medium  or high priority 
(depends on other tasks), and assign it between junior engineers that will 
maintain jQuery pages in the future - so they can do some fixing + learn the 
code. 

Of course once approach will be picked if NCast is going to do some major 
changes to the files, or adding new tabs - they all  will be following the 
right approach :-)


Thanks!
-Pawel





--- On Fri, 5/25/12, Markus Moormann <[email protected]> wrote:

> From: Markus Moormann <[email protected]>
> Subject: Re: [Opencast Matterhorn] Admin jQuery (javaScript) code problem in 
> oc.upload.new.js and other oc.*.js files
> To: "Opencast Matterhorn" <[email protected]>
> Date: Friday, May 25, 2012, 11:53 PM
> Hi Pawel,
> 
> you are right, there are different approaches and they do
> not look the 
> same and I've also ran into that issue. I solved this by
> giving the init 
> function another name. I'm not sure if this issue arises
> because of the 
> different approaches of declaring a "class" in javascript.
> If you are 
> sure about this, go ahead and fix the oc.upload.new.js. But
> then you 
> should think about cleaning all of the files.
> 
> Markus
> 
> Am 25.05.2012 18:15, schrieb Pawel Fic:
> > I was looking at jQuery code (trying to add a
> CAManagement tab -just to learn some of Matterhorn).
> >
> > As Tobias mentioned, I would like to find a problem,
> propose solution, and then introduce it into the project (by
> getting a ticket etc.).
> > So to be constructive:
> >
> >
> >
> >
> > I have researched latest Matterhorn 1.4 (trunk rev
> 12275) of course!
> > And there are 4 different approaches:
> >
> > Approach 1
> > oc.statistics.js:
> > ======================
> > ocStatistics = new (function() ....
> >       this.Component =
> function ...
> >
> > Approach 2
> > oc.admin.js:
> > =============
> > var ocAdmin = (function() {
> >    var admin = {};
> >    admin.Component = ....
> >
> > Approach 3
> > oc.episode.js:
> > ===================
> > var opencast = opencast || {};
> > opencast.episode = (function() {
> >    .....
> >
> > Approach 4
> > oc.upload.js: (BTW: this file is not used any more)
> > ===============
> > var ocUpload = ocUpload || {};
> > ocUpload.init = function()
> >
> >
> >
> >
> >
> >
> > And her is a serious problem I think, as long
> oc.upload.new.js, was written in Approach 2 style:
> >
> > oc.upload.new.js:
> > =========================
> > var ocUpload = (function() {
> >    this.init = function() {
> >        }
> >    return this; ) ;
> >
> > why it causes a problem ?
> > If somebody would like to add a simple tab to
> Matterhorn like (oc.capture_admin_managment.js), s/he would
> start with:
> >
> > var ocCAManager = (function() {
> >    this.init = function() {
> >     .....
> >
> >
> > and there you go! Uploads are ruined by new tab (or
> vice versa depends on the order of includes in index.html).
> >
> >
> > So, my proposal is to :
> > ==============================
> > 1. pick an approach
> > 2. Mark obselete approaches in source code as - not to
> be duplicated (or like: to be changed by newcomers to the
> project).
> >       +instruction how to do
> this.
> > 3. fix oc.upload.new.js  -- I can do this taks.
> >
> >
> > =============
> > What do you think about it ?
> > I
> >
> >
> > _______________________________________________
> > Matterhorn mailing list
> > [email protected]
> > http://lists.opencastproject.org/mailman/listinfo/matterhorn
> >
> >
> > To unsubscribe please email
> > [email protected]
> > _______________________________________________
> >
> >
> _______________________________________________
> Matterhorn mailing list
> [email protected]
> http://lists.opencastproject.org/mailman/listinfo/matterhorn
> 
> 
> To unsubscribe please email
> [email protected]
> _______________________________________________
> 
_______________________________________________
Matterhorn mailing list
[email protected]
http://lists.opencastproject.org/mailman/listinfo/matterhorn


To unsubscribe please email
[email protected]
_______________________________________________

Reply via email to