[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba has been updated. Status: Approved => Merged For more details, see: https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Validation queue job replace-nodes-in-collection-2012-12-14T21-27-02.231Z is finished. The final status was: All tests succeeded! -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Validation queue starting for merge proposal. Log at: http://zorbatest.lambda.nu:8080/remotequeue/replace-nodes-in-collection-2012-12-14T21-27-02.231Z/log.html -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Review: Approve Ghislain didn't vote on his own proposal, but I'm assuming he'd approve it. So I'm adding a second Approve vote in his place. If this is OK, Matthias, go ahead and merge the proposal. -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Review: Approve -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba has been updated. Status: Approved => Needs review For more details, see: https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1, Needs Fixing < 1, Pending < 1. Got: 1 Needs Fixing. -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Validation queue job replace-nodes-in-collection-2012-12-14T13-26-12.575Z is finished. The final status was: All tests succeeded! -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Validation queue starting for merge proposal. Log at: http://zorbatest.lambda.nu:8080/remotequeue/replace-nodes-in-collection-2012-12-14T13-26-12.575Z/log.html -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
> - Is there no more efficient way to find out whether a JSONItem is the root of > a tree or not? Currently, it seems that the searching only works for ordered > collections because it's using findNode. Yes, this makes sense. I introduced Item::isRoot for this. -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
I nevertheless could do some simplification in the code. -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
> - Could the Item::swap function be implemented on JSONObjects, JSONArrays, and > the base-class for all nodes instead of for Item. It seems like that checking > if a node is the root and swapping the tree in XmlNode::swap would be > sufficient. That seems to be more consistent with the way other functions are > implemented. It also seems to avoid the switch in Item::swap and the > replication of raising ZSTR0050. The issue is that access to protected members of Item is needed. They cannot be accessed in XmlNode. -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
> - The comment should mention that you can't replace a node with an object or > vice versa (mentioning inconsistent kind with the ZDDY0040 error is not enough > for the user). Also, that it's only possible to replace root nodes. I updated this accordingly. Replace root nodes only is handled by ZDDY0039. -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
> - I understand why you wanted to rename the function from replace to edit and
> your concern makes sense. However, the description of the function still uses
> the term "replace" and I kind of like the explanation as it is.
Sorry about this, this comment was out of date. I put a description that, I
think, expresses what it does in a more accurate way ("replace" does not seem
right to me as it involves keeping the identity of the content, not of the
target).
--
https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Review: Needs Fixing - The implementation looks very clean. Only a couple of minor comments that might help removing some code but I couldn't find any major rejection points. ;-) - ChangeLog entry is missing - I understand why you wanted to rename the function from replace to edit and your concern makes sense. However, the description of the function still uses the term "replace" and I kind of like the explanation as it is. - The comment should mention that you can't replace a node with an object or vice versa (mentioning inconsistent kind with the ZDDY0040 error is not enough for the user). Also, that it's only possible to replace root nodes. - Is there no more efficient way to find out whether a JSONItem is the root of a tree or not? Currently, it seems that the searching only works for ordered collections because it's using findNode. - Could the Item::swap function be implemented on JSONObjects, JSONArrays, and the base-class for all nodes instead of for Item. It seems like that checking if a node is the root and swapping the tree in XmlNode::swap would be sufficient. That seems to be more consistent with the way other functions are implemented. It also seems to avoid the switch in Item::swap and the replication of raising ZSTR0050. -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba has been updated. Status: Approved => Needs review For more details, see: https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1, Needs Fixing < 1, Pending < 1. Got: 1 Pending. -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Validation queue job replace-nodes-in-collection-2012-12-12T10-42-12.397Z is finished. The final status was: All tests succeeded! -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Validation queue starting for merge proposal. Log at: http://zorbatest.lambda.nu:8080/remotequeue/replace-nodes-in-collection-2012-12-12T10-42-12.397Z/log.html -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
The proposal to merge lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba has been updated. Status: Approved => Needs review For more details, see: https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is requested to review the proposed merge of lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1, Needs Fixing < 1, Pending < 1. Got: 1 Pending. -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is requested to review the proposed merge of lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Validation queue job replace-nodes-in-collection-2012-12-11T19-26-03.976Z is finished. The final status was: All tests succeeded! -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is requested to review the proposed merge of lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
[Zorba-coders] [Merge] lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba
Validation queue starting for merge proposal. Log at: http://zorbatest.lambda.nu:8080/remotequeue/replace-nodes-in-collection-2012-12-11T19-26-03.976Z/log.html -- https://code.launchpad.net/~zorba-coders/zorba/replace-nodes-in-collection/+merge/138196 Your team Zorba Coders is requested to review the proposed merge of lp:~zorba-coders/zorba/replace-nodes-in-collection into lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
