|
Description
Kyle M Hall
2013-04-19 14:28:05 UTC
Created attachment 17556 [details] [review] Bug 10080 - Change system pref IndependantBranches to IndependentBranches Test Plan: 1) Enable IndependantBranches 2) Apply this patch 3) Run updatedatabase.pl 4) Verify that the system preference still functions correctly Created attachment 17588 [details] [review] Bug 10080 - Change system pref IndependantBranches to IndependentBranches Test Plan: 1) Enable IndependantBranches 2) Apply this patch 3) Run updatedatabase.pl 4) Verify that the system preference still functions correctly Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> QA Comment: Tested the db update. Grepped on the old term (only in updatedatabase now). Tested with superlibrarian and normal user in both modes. Nothing unusual. Looks good to me. Jared: Due to number of affected files, this patch would need a lot of rebasing if it stays too long in the queue.. Passed QA I disagree with a change in updatedatabase.pl: There is a modification of an old version ($DBversion = "3.09.00.047"). When this test will be executed, the syspref will not be renamed and will always return 0. Switch back to Passed QA if I missed something. (In reply to comment #4) > I disagree with a change in updatedatabase.pl: > > There is a modification of an old version ($DBversion = "3.09.00.047"). When > this test will be executed, the syspref will not be renamed and will always > return 0. > > Switch back to Passed QA if I missed something. Good catch. Thanks. I agree with you. Kyle: could you send a followup eliminating the change of the older db revision. Only leave the new one under 3.11.00.XXX in updatedatabase. After that please switch back to Signed off. I will test it. This seems like a very dangerous change to make with minimum benefits this close to a major release. I vote it is held over and not rebased until post release. It gains users nothing, with the risk of breaking lots. Created attachment 18044 [details] [review] Bug 10080 - Change system pref IndependantBranches to IndependentBranches Test Plan: 1) Enable IndependantBranches 2) Apply this patch 3) Run updatedatabase.pl 4) Verify that the system preference still functions correctly Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Updated patch, removed modification for the previous database update. Thanks for catching that! I second Chris' vote to hold this patch for 3.14 to not risk missing something, marking bug accordingly. Hi Kyle, your patch contains the po files, but changes to those files should be handled via Pootle. Can you please resubmit your patch without them? I am also not sure if old release notes should be included, as they are 'history'. Created attachment 18155 [details] [review] Bug 10080 - Change system pref IndependantBranches to IndependentBranches Test Plan: 1) Enable IndependantBranches 2) Apply this patch 3) Run updatedatabase.pl 4) Verify that the system preference still functions correctly Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> (In reply to comment #10) > Hi Kyle, > > your patch contains the po files, but changes to those files should be > handled via Pootle. Can you please resubmit your patch without them? > > I am also not sure if old release notes should be included, as they are > 'history'. Good call, fixed! Sorry Kyle, I finally took a step forward in direction of understanding how to grep and found another occurence: /reserve/request.pl. Please provide a follow up. Created attachment 18273 [details] [review] Bug 10080 - Change system pref IndependantBranches to IndependentBranches - Followup QA comment: Looks good to me now. The only occurrences are git grep -l IndependantBranches | grep -v translator installer/data/mysql/updatedatabase.pl misc/release_notes/release_notes_30004.txt misc/release_notes/release_notes_3_10_0.txt misc/release_notes/release_notes_3_12_0.txt We have to keep this patch in mind for future patch! I will provide a patch for the qa tools. Marked as Passed QA. Created attachment 18307 [details] [review] Bug 10080 - Change system pref IndependantBranches to IndependentBranches Test Plan: 1) Enable IndependantBranches 2) Apply this patch 3) Run updatedatabase.pl 4) Verify that the system preference still functions correctly Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 18308 [details] [review] Bug 10080 - Change system pref IndependantBranches to IndependentBranches - Followup Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Pushed to master. Thanks, Kyle! |