Bug 10927 - C4::Utils becomes useless
Summary: C4::Utils becomes useless
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 10925
Blocks:
  Show dependency treegraph
 
Reported: 2013-09-20 15:40 UTC by Jonathan Druart
Modified: 2014-12-07 20:02 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 10927: C4::Utils is unused (4.41 KB, patch)
2013-09-20 15:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10927: C4::Utils is unused (4.45 KB, patch)
2013-09-24 16:26 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED OFF] Bug 10927: C4::Utils is unused (4.47 KB, patch)
2013-09-24 16:29 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 10927: C4::Utils is unused (4.57 KB, patch)
2013-10-03 12:35 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2013-09-20 15:40:12 UTC
Bug 10925 removes the last use to C4::Utils.
We can remove it.
Comment 1 Jonathan Druart 2013-09-20 15:49:34 UTC Comment hidden (obsolete)
Comment 2 I'm just a bot 2013-09-24 05:41:12 UTC
Patch applied cleanly, go forth and signoff
Comment 3 I'm just a bot 2013-09-24 05:41:38 UTC
Patch applied cleanly, go forth and signoff
Comment 4 Mark Tompsett 2013-09-24 15:44:55 UTC
$ grep -l hashdump `find .`
./C4/Auth_with_ldap.pm
./C4/Utils.pm
./t/db_dependent/Context.t

$ grep -l maxwidth `find .`
./C4/Utils.pm
./C4/Images.pm
./t/db_dependent/Context.t

Checking C4/Images.pm showed that wasn't applicable.
Checking C4/Auth_with_ldap.pm showed that it was applicable.

I think it is triggered by the LDAP user existing locally and debug turned on, but I didn't try to figure out expressly.

I eyeballed the patch and was wondering why the "Examining defined key values" section was cut, and not changed. Is there a reason to cut it? Perhaps it makes the test look prettier. :)
Comment 5 Jonathan Druart 2013-09-24 15:55:40 UTC
Did you apply dependent patches?
Comment 6 Mark Tompsett 2013-09-24 16:05:11 UTC
Okay, that solves the mystery of the Auth_with_ldap.pm, but that doesn't answer the question about why cut the "Examining defined key values" section.
Comment 7 Mark Tompsett 2013-09-24 16:26:46 UTC Comment hidden (obsolete)
Comment 8 Mark Tompsett 2013-09-24 16:29:01 UTC Comment hidden (obsolete)
Comment 9 Mark Tompsett 2013-09-24 17:19:00 UTC
$ git bz apply 10925
$ git bz apply 10927
$ prove -v t/db_dependent/Context.t

I also confirmed that t/Utils.t was removes, and C4/Utils.pm was as well.

I did not do an LDAP login. However, that should be tested with 10925.

I did not do a CAS login. However, this eyeballs rather easily, in my opinion.
Comment 10 Jonathan Druart 2013-09-25 07:12:23 UTC
(In reply to M. Tompsett from comment #6)
> Okay, that solves the mystery of the Auth_with_ldap.pm, but that doesn't
> answer the question about why cut the "Examining defined key values" section.

Hello Mark,
I removed the section because it was only a display. Being into a unit test file, I don't think it is useful to keep it.
Comment 11 Katrin Fischer 2013-09-28 11:01:02 UTC
This is BLOCKED, because 10925 still waits for sign-off.
Comment 12 Mark Tompsett 2013-09-28 15:51:29 UTC
Perhaps it is premature, but I switched this to signed off, as I got around to testing 10925 and signing it off.
Comment 13 Katrin Fischer 2013-09-28 16:28:06 UTC
Please leave at BLOCKED, it can go back to Signed off when the dependency is resolved.
Comment 14 Jonathan Druart 2013-09-30 07:40:15 UTC
(In reply to Katrin Fischer from comment #13)
> Please leave at BLOCKED, it can go back to Signed off when the dependency is
> resolved.

Hi Katrin,
I'm not sure to understand : if the dependency is signed off, I think this one could be have a signed off status too.
On this way, 2 patches could be QA together, don't you think?
Comment 15 Katrin Fischer 2013-09-30 08:33:37 UTC
Hm, I thought it was Failed QA when I checked - so there was not much one could do here.
Comment 16 Kyle M Hall 2013-10-03 12:35:40 UTC
Created attachment 21761 [details] [review]
Bug 10927: C4::Utils is unused

Bug 10925 removes the last call to C4::Utils.
The module becomes useless and can be deleted.

Verify that t/db_dependent/Context.t still successfully passes.
git grep hashdump
git grep maxwidth

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passes koha-qa.pl, no subs from the module are used anywhere
Comment 17 Galen Charlton 2013-10-03 14:58:36 UTC
Pushed to master.  Thanks, Jonathan!