[Bug 9006] New: autoload in C4::Context is a bad idea
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 Priority: P5 - low Change sponsored?: --- Bug ID: 9006 Assignee: gmcharlt@gmail.com Summary: autoload in C4::Context is a bad idea Severity: minor Classification: Unclassified OS: All Reporter: jcamins@cpbibliography.com Hardware: All Status: NEW Version: master Component: Architecture, internals, and plumbing Product: Koha The comment in C4/Context.pm pretty much sums it up: # AUTOLOAD # This implements C4::Config->foo, and simply returns # C4::Context->config("foo"), as described in the documentation for # &config, above. # FIXME - Perhaps this should be extended to check &config first, and # then &preference if that fails. OTOH, AUTOLOAD could lead to crappy # code, so it'd probably be best to delete it altogether so as not to # encourage people to use it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 37417 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37417&action=edit Bug 9006: Remove AUTOLOAD in C4::Context Happily this was only used for intranetdir. It's time to remove it and replace existing calls. I used the following commands to catch calls to C4::Context: git grep 'C4::Context\->' | grep -v 'C4::Context->preference' | grep -v 'C4::Context->config' | grep -v 'C4::Context->userenv' | grep -v 'C4::Context->IsSuperLibrarian' | grep -v 'C4::Context->dbh' | grep -v 'C4::Context->set_preference' | grep -v '_syspref_cache' | grep -v _userenv | grep -v 'C4::Context->interface' | grep -v 'C4::Context->Zconn' | grep -v 'C4::Context->queryparser' | grep -v 'C4::Context->tz' | grep -v 'C4::Context->boolean_preference' | grep -v 'C4::Context->memcached' -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m Assignee|gmcharlt@gmail.com |jonathan.druart@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com --- Comment #2 from M. Tompsett <mtompset@hotmail.com> --- $ grep "^sub " C4/Context.pm | cut -f2 -d' ' > ~/subs $ vi ~/subs :%s/^/C4::Context->/g :wq $ git grep 'C4::Context\->' | grep -v -f ~/subs Yes, intranetdir, but also: 1 perldoc line IsSuperlibrarian typo something ugly in C4/Input.pm typo in 3.07.00.030 updatedatabase.pl upgrade Perhaps those deserve a bug report? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #3 from M. Tompsett <mtompset@hotmail.com> --- Koha::FrameworkPlugin needs tweaking, and is the reason a lot of the other stuff doesn't apply. Please rebase, amend, test, and reattach for signoff. :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #37417|0 |1 is obsolete| | --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 40349 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40349&action=edit Bug 9006: Remove AUTOLOAD in C4::Context Happily this was only used for intranetdir. It's time to remove it and replace existing calls. I used the following commands to catch calls to C4::Context: git grep 'C4::Context\->' | grep -v 'C4::Context->preference' | grep -v 'C4::Context->config' | grep -v 'C4::Context->userenv' | grep -v 'C4::Context->IsSuperLibrarian' | grep -v 'C4::Context->dbh' | grep -v 'C4::Context->set_preference' | grep -v '_syspref_cache' | grep -v _userenv | grep -v 'C4::Context->interface' | grep -v 'C4::Context->Zconn' | grep -v 'C4::Context->queryparser' | grep -v 'C4::Context->tz' | grep -v 'C4::Context->boolean_preference' | grep -v 'C4::Context->memcached' -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14422 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14425 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=13876 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 --- Comment #5 from M. Tompsett <mtompset@hotmail.com> --- I re-ran my commands in comment #2 $ grep "^sub " C4/Context.pm | cut -f2 -d' ' > ~/subs $ vi ~/subs :%s/^/C4::Context->/g :wq $ git grep 'C4::Context\->' | grep -v -f ~/subs Yes, intranetdir, but also: 1 perldoc line IsSuperlibrarian typo (see also bug 14425) something ugly in C4/Input.pm (patch to come on this bug) typo in 3.07.00.030 updatedatabase.pl upgrade (see also bug 14422) But I discovered that bug 13876 introduced C4::Context->KOHAVERSION. I think it should 'use Koha;' and then be 'Koha::version'; Also, this likely should be a patch on this bug. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=13758 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14427 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14428 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- (In reply to M. Tompsett from comment #5)
something ugly in C4/Input.pm (patch to come on 14428, though perhaps C4::Input should be removed) ... But I discovered that bug 13876 introduced C4::Context->KOHAVERSION. I think it should 'use Koha;' and then be 'Koha::version'; This is on bug 14427.
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |BLOCKED Depends on| |14428 --- Comment #7 from M. Tompsett <mtompset@hotmail.com> --- Setting this to depends on 14428, because 14428 removes the one non-intranetdir autoload by removing C4::Input (good bye, crud!). Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14428 [Bug 14428] Remove C4::Input -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Needs Signoff --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to M. Tompsett from comment #7)
Setting this to depends on 14428, because 14428 removes the one non-intranetdir autoload by removing C4::Input (good bye, crud!).
This cans still be signed off. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40349|0 |1 is obsolete| | --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 40916 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40916&action=edit Bug 9006: Remove AUTOLOAD in C4::Context Happily this was only used for intranetdir. It's time to remove it and replace existing calls. I used the following commands to catch calls to C4::Context: git grep 'C4::Context\->' | grep -v 'C4::Context->preference' | grep -v 'C4::Context->config' | grep -v 'C4::Context->userenv' | grep -v 'C4::Context->IsSuperLibrarian' | grep -v 'C4::Context->dbh' | grep -v 'C4::Context->set_preference' | grep -v '_syspref_cache' | grep -v _userenv | grep -v 'C4::Context->interface' | grep -v 'C4::Context->Zconn' | grep -v 'C4::Context->queryparser' | grep -v 'C4::Context->tz' | grep -v 'C4::Context->boolean_preference' | grep -v 'C4::Context->memcached' NOTE: I applied 14428, and then did what I suggested in comment #2. Only intranetdir references appeared. I applied this patch, and repeated. Nothing appeared. This means the autoload references are properly removed. koha qa test tools complained about whitespace, I just fixed those. Though, we may wish to perltidy auth_fields_substructure.pl on another bug. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40916|0 |1 is obsolete| | --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 40924 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40924&action=edit [PASSED QA] Bug 9006: Remove AUTOLOAD in C4::Context Happily this was only used for intranetdir. It's time to remove it and replace existing calls. I used the following commands to catch calls to C4::Context: git grep 'C4::Context\->' | grep -v 'C4::Context->preference' | grep -v 'C4::Context->config' | grep -v 'C4::Context->userenv' | grep -v 'C4::Context->IsSuperLibrarian' | grep -v 'C4::Context->dbh' | grep -v 'C4::Context->set_preference' | grep -v '_syspref_cache' | grep -v _userenv | grep -v 'C4::Context->interface' | grep -v 'C4::Context->Zconn' | grep -v 'C4::Context->queryparser' | grep -v 'C4::Context->tz' | grep -v 'C4::Context->boolean_preference' | grep -v 'C4::Context->memcached' NOTE: I applied 14428, and then did what I suggested in comment #2. Only intranetdir references appeared. I applied this patch, and repeated. Nothing appeared. This means the autoload references are properly removed. koha qa test tools complained about whitespace, I just fixed those. Though, we may wish to perltidy auth_fields_substructure.pl on another bug. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org