https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16008 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16008 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |mtompset@hotmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16008 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48830|0 |1 is obsolete| | --- Comment #3 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 48862 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48862&action=edit Bug 16008: noisy "fatal" warning when new file is moved in another patch The failure was tracked down to check_forbidden_patterns in QohA/File.pm's check_forbidden_patterns function. As there can't be any forbidden patterns on a non-existent file, this patch returns the default 1 when the file being checked does not exist. It maintains a warning message as suggested in comment #2. It is better to create another check as stated in comment #2, but I figure I'll share my less than perfect work anyways. TEST PLAN --------- apply bug 15870 to your koha run qa test tools -- see the warning apply this patch to your qa test tools run qa test tools -- no more warning -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16008 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 49318 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49318&action=edit [COUNTER-PATCH] Bug 16008: Deal with removed files Bug 16008: Deal with removed files This patch fixes 2 issues: 1/ When a patch just removes files, the $git->log method did not return the right list of files. The commit which only removed files were not counted. 2/ If a file is added then deleted in the same patch set, the following error occurred: fatal: ambiguous argument 't/db_dependent/RecordProcessor_ViewPolicy.t': unknown revision or path not in the working tree. Now it's correctly handled: a new 'git manipulation' check has been added. Note that lot of changes has been made to achieve this goal: 1/ QohA::Files->filter has been removed: it was not needed the assignement was done in the BUILDER 2/ QohA::Git->log now returns all changes (even for deleted files) 3/ QohA::Report->type has been deleted, it was not used 4/ The report member initialization has been moved to the parent class (QohA::File). Test plan: % echo "#simple file" > new_file % echo "#comment" >> mainpage.pl % git add new_file mainpage.pl; git commit -m'1' % git rm new_file % git commit -m'2' % qa -c 2 -v 2 # should return a warning for new_file -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16008 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=16104 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16008 --- Comment #8 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Jonathan Druart from comment #7)

Mark, any chances to see you testing this patch?

I'll let QA test this harder, but I have tested for the newer git manipulation test which you added (nice simplification) and forbidden patterns (since I hit that with 16236 last night). Spelling, tt_valid, and valid_template failures should still be confirmed. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16008 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48862|0 |1 is obsolete| | Attachment #49318|0 |1 is obsolete| | --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 50136 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=50136&action=edit [SIGNED-OFF] Bug 16008: Deal with removed files This patch fixes 2 issues: 1/ When a patch just removes files, the $git->log method did not return the right list of files. The commit which only removed files were not counted. 2/ If a file is added then deleted in the same patch set, the following error occurred: fatal: ambiguous argument 't/db_dependent/RecordProcessor_ViewPolicy.t': unknown revision or path not in the working tree. Now it's correctly handled: a new 'git manipulation' check has been added. Note that lot of changes has been made to achieve this goal: 1/ QohA::Files->filter has been removed: it was not needed the assignement was done in the BUILDER 2/ QohA::Git->log now returns all changes (even for deleted files) 3/ QohA::Report->type has been deleted, it was not used 4/ The report member initialization has been moved to the parent class (QohA::File). Test plan: % echo "#simple file" > new_file % echo "#comment" >> mainpage.pl % git add new_file mainpage.pl; git commit -m'1' % git rm new_file % git commit -m'2' % qa -c 2 -v 2 # should return a warning for new_file NOTE: ran this test plan, and checked forbidden patterns. need confirmation on spelling, tt_valid, and valid_template. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16008 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Too bad not to see another QAer taking a look at this one. I am going to push it anyway. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16008 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #11)

Too bad not to see another QAer taking a look at this one. I am going to push it anyway.

No objection at all for doing so. If there would be an error, you will hear :) Fetched latest commit now.. ;) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16008 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #13)

Now tested again with Bug 15839 Error is gone, but I see a few 0's and 1's (debug stuff?)

OK C4/Review.pm OK critic OK forbidden patterns 0 OK git manipulation OK pod SKIP spelling OK valid 0

FAIL Koha/Review.pm OK critic OK forbidden patterns 1

Yes I have seen it too, but I think it already existed prior to this patch. -- You are receiving this mail because: You are watching all bug changes.