openerp-community-reviewer team mailing list archive
-
openerp-community-reviewer team
-
Mailing list archive
-
Message #02582
Re: [Merge] lp:~serpentcs/server-env-tools/7.0-base_synchro into lp:server-env-tools
Review: Disapprove
In actual fact that is exactly what Nishant did. Maybe you see a different diff to me and my work has been respected. I know nothing of any concept of slicing, and certainly I did nothing to disrespect Nishants prior work.
Anyway, the merge proposal belongs as a seperate proposal because of the following
1. The new merge proposal respect prior work and does not steal - having this merged sets a very bad precedent. In any case, by cutting and pasting it removes the history of commits which is important. Also as it happens the cut and paste job was poorly done.
2. Changes more than 50% of the preceding code requiring fresh eyes and fresh reviews - minor adjustments in a merge proposal are acceptable, but when a new author makes such large scale changes, a total refactor, it really requires a restart of the process. It is not incremental change.
3. Any recomendations for improvement will likely fall on me to fix. This is far easier on a branch I can push to.
4. The level of testing performed before this proposal was made was abysmal, if any, relying on community to find very basic bugs. It didn't work at all beyond installing and could have caused major data issues for anyone installing in production. Community time is short and expecting community to perform exhaustive testing because the proposer hasn't is disrespectful of our time.
4. The new branch merges without conflicts.
5. The new branch adds a new module in addition to base_synchro which may or may not be merged in to the main module.
6. Having an MP owner who doesn't really understand the code, or wrote the code is a nonsense. They must be able to respond to reviewer questions. In essence this module was a direct port of OpenERP 6.1 code with some syntactic changes to make sure it installed and a copyright message slapped on it. It really didn't go through any quality process I can see prior to the merge proposal being made.
Thanks for your Understanding
> Graeme,
>
> No doubt, no one take your work and pass it off with a thanks note.
>
> We are using the concept of slicing inspired by you. Don't take it otherwise.
> The purpose is just to submit solutions to OpenERP commnity. No hard feelings.
>
> Rather than adding a new proposal, we would like to you propose a merge into
> this parent proposal and help community to improve it and we all can work on
> one proposal.
>
> Nishant, please merge Graeme's proposal (should come soon) respecting all his
> copyright and credits to him. His recent changes are really better than us and
> satisfying all quality standards.
>
> Thank you for understanding, lets work towards solutions.
>
> Have a good day.
--
https://code.launchpad.net/~serpentcs/server-env-tools/7.0-base_synchro/+merge/183102
Your team Server Environment And Tools Core Editors is subscribed to branch lp:server-env-tools.
References