Fwd: wip/new-model review

(resending to the list because moderation failed. Colin, some extra notes are added)

On Sun, Jul 7, 2013 at 11:26 AM, Colin Walters <walters verbum org> wrote:
Hello ostree-list,

Jasper made a branch "wip/new-model" which will ensure that we only have
one version number per build, among other things.  Sending to the list
for general interest.

So far, I've cherry-picked a few patches to master.  The first problem I
hit was with:
was actually using imports.JsonDB.  Fixed that and pushed.  BTW
did you use any script to do the imports analysis?

I just used C-s in emacs to look for results, so it's not complete. Sorry about that, though :(
Next, with:
At the moment the ostree.modules jhbuild moduleset is sticking to GNOME
3.8 branches, and there js185 does *not* have String.endsWith.  We could
monkey patch it in though.

Yeah, OK, I'll drop this one for now, but perhaps a polyfill for older environments (in gjs even?) is a good idea.

Can you explain:
What symlink was being concurrently modified?

I think at one time I was going to use atomicSymlinkSwap from a forked-off task, so if two tasks tried to symlink at the same time, they could overwrite each other's "current-tmp", but I dropped that, so it's not really needed. Note that I think that the commit is correct anyway (current-tmp is completely bogus), but I dropped it for now.
This one is interesting:
One problem though is now multiple queueResolve() will overwrite the
previous one.  So if we get two git push notifications while a resolve
is running, we'll only fetch the second one.

This line is incorrect:
  + let name = matches[i]['name'];
Should be matches[j].  (I really wish JS had a better array iteration

That cleans up the code considerably.
- if (this.parameters.fetchAll || this.parameters.fetchComponents.length > 0) {
+ if (this.parameters.fetchAll || componentsToFetch.length > 0) {

You left the "fetchComponents" parameter in the list, but it now does
nothing.  I actually use it from the command line now, like:

$ ostbuild make resolve fetchComponents='["glib"]'


        let componentsToFetch = this.parameters.fetchComponents.slice();
Am I missing something here, or does componentsToFetch not start out as the fetchComponents list?

Can you just add a componentsToFetch.push.apply(componentsToFetch,
this.parameters.fetchComponents) ?

I'll get to the others in a bit.  I'm hoping to land this weekend the
deployment 2.0 model branches for various git modules.

I've pushed a reworked set of patches for this fixing most of the other issues to wip/new-model.



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]