Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/1025564 into lp:zorba
Review: Approve I'm not sure why Launchpad is showing hundreds of changes on this branch; something odd when merging from the trunk, I assume. But the diff looks good, so if it merges OK, I'm happy. -- https://code.launchpad.net/~zorba-coders/zorba/1025564/+merge/125256 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/1025564 into lp:zorba
I'm not sure why Launchpad is showing hundreds of changes on this branch; something odd when merging from the trunk, I assume. But the diff looks good, so if it merges OK, I'm happy. Since most of the changes are in bin folder, that has not been changed in a while now, just to be 100% sure that everything is OK one could: - create a new branch of lp:zorba locally - merge the proposed changes in this branch and propose it for merging (be sure to give it a unique name when pushing the changes to LP) - delete this merge proposal and related branch -- https://code.launchpad.net/~zorba-coders/zorba/1025564/+merge/125256 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/1025564 into lp:zorba
Review: Needs Information What happened to Fixed bug #866958 (Parsing error not explicit enough) in the ChangeLog? -- https://code.launchpad.net/~zorba-coders/zorba/1025564/+merge/125256 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/1025564 into lp:zorba
That bug was never part of this proposal. I think it's showing up in the did here due to my merge from the trunk. The Changelog had a number of things get rearranged which made conflict resolution hard. That bug is in fact fixed so I assume it should be in the Changelog. -- https://code.launchpad.net/~zorba-coders/zorba/1025564/+merge/125256 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/1025564 into lp:zorba
Review: Approve -- https://code.launchpad.net/~zorba-coders/zorba/1025564/+merge/125256 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/1025564 into lp:zorba
The attempt to merge lp:~zorba-coders/zorba/1025564 into lp:zorba failed. Below is the output from the failed tests. CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275 (message): Validation queue job 1025564-2013-04-30T22-40-03.37Z is finished. The final status was: 1 tests did not succeed - changes not commited. Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake -- https://code.launchpad.net/~zorba-coders/zorba/1025564/+merge/125256 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/1025564 into lp:zorba
There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions. -- https://code.launchpad.net/~zorba-coders/zorba/1025564/+merge/125256 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/1025564 into lp:zorba
Comments fixed, please re-review. -- https://code.launchpad.net/~zorba-coders/zorba/1025564/+merge/125256 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/1025564 into lp:zorba
Review: Needs Fixing Also, add a mention to the option changes to the ChangeLog. -- https://code.launchpad.net/~zorba-coders/zorba/1025564/+merge/125256 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/1025564 into lp:zorba
Review: Needs Fixing
Works great. Some minor issues:
- the --help documentation should mention that the argument is deprecated
- also, the --help documentation should mention the syntax to pass the file
- please respect coding guidelines. For example,
if(qBeforeF()){ = if (qBeforeF())
{
- remove commented out code
--
https://code.launchpad.net/~zorba-coders/zorba/1025564/+merge/125256
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/1025564 into lp:zorba
Agree about the --help doc; if I get a little time later I might put in a suggested change myself. Also agree about not leaving commented-out code. To be fair to Luis, the coding guidelines do not mention anything about brace positioning or spaces after if, etc. -- https://code.launchpad.net/~zorba-coders/zorba/1025564/+merge/125256 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
