[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r20991 - trunk/contrib/hook-scripts

From: Bhuvaneswaran Arumugam <bhuvan_at_collab.net>
Date: 2006-08-06 05:35:07 CEST

> Please try to avoid cc-ing svn@s.t.o - it creates needless duplicate
> messages.

Yeah, i'll avoid it!

>
> >> - if(childNode.nodeName == 'author'):
> >> - self.feed.insertBefore(item, childNode.nextSibling)
> >> - elif(childNode.nodeName == 'entry'):
> >> + if childNode.nodeName == 'entry':
> >> + if total == 0:
> >> + self.feed.insertBefore(item, childNode)
> >> + total += 1
> >> total += 1
> >
> > Weird! We increment the "total" twice when the value is 0. Result: We
> > manage to store max_items-1 items in the feed file. I bet, you intend to
> > remove the increment statement which resides inside the "if total == 0:"
> > loop :)
>
> No, I meant exactly what I committed.
> You should count both the <entry> node you have just seen, and the one
> you are inserting. 1 + 1 = 2.
>
> Or at least, that's the idea. It doesn't work, though, because it seems
> that it is not safe to iterate over a childNodes collection whilst
> changing it.

Nope! Here, the loop is executed separately for the item we insert using
"insertBefore" function and it does not bother to iterate over a
childNodes collection whilst changing it. In current state, when we run
the script as below, it inserts only 9 items instead of 10 items.

$ ./svn2feed.py -F atom -u .. -U .. -r1:10 /path/to/repos/ -m 10
# remove the feed and pickle files before you run the script.

In this case, a temporary print statement for "total" variable explains
that the loop is iterated 10 times (while processing last revision) and
the "total" variable is set as 11, so, it removes an item
unnecessarily.

I'll be happy as long as the script works as expected!

> > Here again, why do we wish to include the above "if" condition for every
> > childNode ? We call this function for every revision, so it's good to
> > place it outside the "for" loop. Moreover, in the current state it'll
> > not remove multiple nodes (if you intend to), since we increment the
> > "total" one at a time.
>
> No, it is not good to place it outside the for loop; it would not then
> be able to remove multiple nodes.
> I believe it *will* remove multiple nodes if needed - I don't understand
> your explanation of why you think it won't.

My point is, we call this function for every revision. So, it's fine to
remove one item at a time. Moreover, i'm certain the condition is met
only when processing the last element, so, why we need to check the
condition for all child nodes.

Thank you very much for looking into it. I really appreciate your
guidance and help.

-- 
Regards,
Bhuvaneswaran

Received on Sun Aug 6 05:36:01 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.