https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408
--- Comment #93 from Arthur Suzuki <[email protected]> --- (In reply to Victor Grousset/tuxayo from comment #90) > > - Unnecessary changes are made e.g. $patron->borrowernumber > > to $patron->id (correct me if I'm wrong here, please!) > > Indeed, in bunch of lines that are already changed, it's a good opportunity > to enforce terminology without adding more conflict possibilities. But here > it's in lines that are not changed, so it's adding new conflicts > possibilities with other submitted patches and when backporting. I've fixed this in the latest version of the patch :) > > - Some lines that have not been changed have been tidied and included > > Yes, also that. Also fixed :) > > - Personally I'd prefer this to be unfolded in more commits > > to ease the review process, with each having a short > > explanation of why the changes are required: > > At least having the full commit message list these points. Not sure about > splitting the commit after the work is done. Maybe I missed the main point, > but in this case the 3 changes look 95% likely to have been done together > and not as iteration with a different logic on each step. > That's a nice thing to keep in mind for other reviews or my own patches, > thanks :) I've added the relevant points to the commit message, next time I will try to better split the commits. I don't know if the patch can move to "Passed QA" or if further work is needed? Thanks for the review anyway :) Best, Arthur -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
