Bug 16011

Summary: Remove $VERSION from our modules
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: brendan, colin.campbell, gmcharlt, josef.moravec, katrin.fischer, tomascohen
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 16011: $VERSION - remove use vars $VERSION
Bug 16011: $VERSION - remove use vars $VERSION
Bug 16011: $VERSION - Remove the $VERSION init
Bug 16011: $VERSION - Remove comments
Bug 16011: $VERSION - remove use vars $VERSION
Bug 16011: $VERSION - Remove the $VERSION init
Bug 16011: $VERSION - Remove comments
Bug 16011: $VERSION - Remove use vars qw();
Bug 16011: $VERSION - Remove empty BEGIN block
Bug 16011: $VERSION - Remove use vars qw();
Bug 16011: $VERSION - Remove empty BEGIN block
Bug 16011: $VERSION - remove use vars $VERSION
Bug 16011: $VERSION - Remove the $VERSION init
Bug 16011: $VERSION - Remove comments
Bug 16011: $VERSION - Remove use vars qw();
Bug 16011: $VERSION - Remove empty BEGIN block
Bug 16011: Reintroduce $VERSION for 2 pm
Bug 16011: $VERSION - remove use vars $VERSION
Bug 16011: $VERSION - Remove the $VERSION init
Bug 16011: $VERSION - Remove comments
Bug 16011: $VERSION - Remove use vars qw();
Bug 16011: $VERSION - Remove empty BEGIN block
Bug 16011: Reintroduce $VERSION for 2 pm
Patch for missing declartaion in BakerTaylor
Bug 16011 reintroduced VERSION variable needs declaration

Description Jonathan Druart 2016-03-09 08:42:59 UTC
It has been decided to remove the $VERSION from our modules.
See the discussion on http://koha.1045719.n5.nabble.com/Proposal-Deal-with-modules-versioning-VERSION-td5876752.html
Comment 1 Jonathan Druart 2016-03-09 08:49:32 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-03-09 08:50:24 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-03-09 08:50:30 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-03-09 08:50:37 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2016-03-09 08:52:26 UTC
Intentionally there is no test plan for this patch set.
Comment 6 Josef Moravec 2016-03-09 09:33:52 UTC
Shouldn't be the whole line removed in these files?

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48836&action=diff#a/C4/BackgroundJob.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48836&action=diff#a/C4/ClassSortRoutine/Dewey.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48836&action=diff#a/C4/ClassSortRoutine/Generic.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48836&action=diff#a/C4/ClassSortRoutine/LCC.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48836&action=diff#a/C4/Matcher.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48836&action=diff#a/C4/Members/AttributeTypes.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48836&action=diff#a/C4/Members/Messaging.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48836&action=diff#a/C4/SMS.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48836&action=diff#a/Koha/Borrower/Files.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48836&action=diff#a/Koha/Misc/Files.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48836&action=diff#a/Koha/Template/Plugin/Cache.pm_sec1

For you it's just another regexp I believe ;)


Here stayd empty BEGIN block, is it necessary to keep it? Just asking ;)

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48838&action=diff#a/C4/BackgroundJob.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48838&action=diff#a/C4/Matcher.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48838&action=diff#a/C4/Members/AttributeTypes.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48838&action=diff#a/C4/Members/Messaging.pm_sec1

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48838&action=diff#a/Koha/Borrower/Files.pm_sec1
Comment 7 Jonathan Druart 2016-03-09 12:42:48 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2016-03-09 12:42:52 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2016-03-09 12:42:55 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2016-03-09 12:42:59 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2016-03-09 12:43:02 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2016-03-09 12:43:44 UTC
(In reply to Josef Moravec from comment #6)
> Shouldn't be the whole line removed in these files?

> Here stayd empty BEGIN block, is it necessary to keep it? Just asking ;)

Yep agreed! :)
Comment 13 Josef Moravec 2016-03-09 14:18:44 UTC
I think there are probably two more use vars qw( ); statements (note the space between parenthesis) in files: 

Koha/Template/Plugin/Cache.pm

C4/SMS.pm
Comment 14 Galen Charlton 2016-03-09 18:07:11 UTC
Upon initial review, this looks like it's on the right track and  t/db_dependent/00-strict.t passes.

I note that Koha::SuggestionEngine::* plugins include a ->VERSION() method, but that's not strictly on point for this particular bug.
Comment 15 Jonathan Druart 2016-03-10 12:39:29 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2016-03-10 12:39:33 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2016-03-10 12:39:52 UTC
(In reply to Josef Moravec from comment #13)
> I think there are probably two more use vars qw( ); statements (note the
> space between parenthesis) in files: 
> 
> Koha/Template/Plugin/Cache.pm
> 
> C4/SMS.pm

Patch amended.
Comment 18 Josef Moravec 2016-03-10 13:15:41 UTC Comment hidden (obsolete)
Comment 19 Josef Moravec 2016-03-10 13:15:50 UTC Comment hidden (obsolete)
Comment 20 Josef Moravec 2016-03-10 13:15:55 UTC Comment hidden (obsolete)
Comment 21 Josef Moravec 2016-03-10 13:16:01 UTC Comment hidden (obsolete)
Comment 22 Josef Moravec 2016-03-10 13:16:09 UTC Comment hidden (obsolete)
Comment 23 Katrin Fischer 2016-03-20 22:32:08 UTC
The QA tools report some problems:

 FAIL	t/Koha/Plugin/Test.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  pod
   OK	  spelling
   FAIL	  valid
		Global symbol "$VERSION" requires explicit package name 
		t/Koha/Plugin/Test.pm had compilation errors.

 FAIL	C4/External/BakerTaylor.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  pod
   OK	  spelling
   FAIL	  valid
		C4/External/BakerTaylor.pm had compilation errors.
		Global symbol "$VERSION" requires explicit package name 

Can you please check?
Comment 24 Jonathan Druart 2016-03-21 14:20:11 UTC Comment hidden (obsolete)
Comment 25 Tomás Cohen Arazi 2016-03-23 15:07:06 UTC
Created attachment 49441 [details] [review]
Bug 16011: $VERSION - remove use vars $VERSION

perl -p -i -e 's/^(use vars .*)\$VERSION\s?(.*)/$1$2/' **/*.pm

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Comment 26 Tomás Cohen Arazi 2016-03-23 15:07:13 UTC
Created attachment 49442 [details] [review]
Bug 16011: $VERSION - Remove the $VERSION init

Mainly a
  perl -p -i -e 's/^.*3.07.00.049.*\n//' **/*.pm
Then some adjustements

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Comment 27 Tomás Cohen Arazi 2016-03-23 15:07:18 UTC
Created attachment 49443 [details] [review]
Bug 16011: $VERSION - Remove comments

perl -p -i -e 's/^.*set the version for version checking.*\n//' **/*.pm

+ manual adjustements

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Comment 28 Tomás Cohen Arazi 2016-03-23 15:07:26 UTC
Created attachment 49444 [details] [review]
Bug 16011: $VERSION - Remove use vars qw();

perl -p -i -e 's/use vars qw\(\s*\);\n//' **/*.pm

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Comment 29 Tomás Cohen Arazi 2016-03-23 15:07:34 UTC
Created attachment 49445 [details] [review]
Bug 16011: $VERSION - Remove empty BEGIN block

perl -p -i -0 -e 's/BEGIN \{\n?\n?\}\n//' **/*.pm

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Comment 30 Tomás Cohen Arazi 2016-03-23 15:07:40 UTC
Created attachment 49446 [details] [review]
Bug 16011: Reintroduce $VERSION for 2 pm

These files should be managed on their own bug reports.

Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Comment 31 Brendan Gallagher 2016-03-24 17:25:48 UTC
Pushed to Master - Should be in the May 2016 release.  Thanks!
Comment 32 Colin Campbell 2016-03-29 12:42:05 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2016-03-29 13:36:58 UTC
Created attachment 49633 [details] [review]
Bug 16011 reintroduced VERSION variable needs declaration

$VERSION reintroduced into External/BakerTaylor.pm but was
not declared. Causes tests to fail.
Readd VERSION to the package vars

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 34 Brendan Gallagher 2016-03-29 22:39:38 UTC
(In reply to Jonathan Druart from comment #33)
> Created attachment 49633 [details] [review] [review]
> Bug 16011 reintroduced VERSION variable needs declaration
> 
> $VERSION reintroduced into External/BakerTaylor.pm but was
> not declared. Causes tests to fail.
> Readd VERSION to the package vars
> 
> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Last patch pushed - Mar 29th 2016