cozy-labs / cozy-light

Personal Server Made Easy
http://cozy-labs.github.io/cozy-light/
GNU Affero General Public License v3.0
104 stars 13 forks source link

code style #56

Closed maboiteaspam closed 9 years ago

maboiteaspam commented 9 years ago

Hi,

Small thing i'd like talk about,

in the core code we can find many

        if (callback !== undefined && typeof(callback) === 'function') {
          callback();
        }

How about moving that to nodeHelper as

call:function(cb,args1,args2,argsN){
        if (cb !== undefined && typeof(cb) === 'function') {
          cb.apply(null,arguments);
        }
}

Also, i see now we do this,

        if (configHelpers.mainWatcher !== undefined) {
          configWatcher.one(callback);
        }

I think we figure out the same problem. Except that i understood that as a lack of status representation internally. I mean that internally, we don t keep track of the current status (started, stopped, restarting), neither about current action such install / uninstall-app etc. Thing is nothing prevent to enter in weird behavior if twice of those events occurs together.

Downside is the extra code to support that, but seems like somehow it has to be managed, as of this new code shown.

Loking for some thoughts.

frankrousseau commented 9 years ago

+1 for the call function but we shouldn't call it call because this name is already used by the JS standard lib.

About the watcher problem, I think it works well now. Let's try using it that way. If we meet another problems, I agree we will have to make things better architectured. I lost too much time trying to fix that behavior.

frankrousseau commented 9 years ago

Let's say it's done.