https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408
Victor Grousset/tuxayo <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #90 from Victor Grousset/tuxayo <[email protected]> --- > - 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. > - Some lines that have not been changed have been tidied and included Yes, also that. > - 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 :) -- 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/
