Koha::Object depends on Koha::DateUtils, which depends on C4::Context, which depends on Koha::Config::SysPref, which depends on... Koha::Object
Created attachment 163940 [details] [review] Bug 36432: Remove circular dependency from Koha::Object Koha::Object depends on Koha::DateUtils, which depends on C4::Context, which depends on Koha::Config::SysPref, which depends on... Koha::Object Apart from the circular dependency, the dependency on C4::Context alone is problematic as it loads a bunch of modules that are not needed at all in Koha::Object (YAML::XS and ZOOM for instance). As Koha::Object is used as a base for a lot of modules, we should take care to only load the minimum required. This patch moves some date parsing code to specific modules: - Koha::DateTime::Format::RFC3339 - Koha::DateTime::Format::SQL and it uses them in Koha::Object and Koha::DateUtils where it is possible. Test plan: 1. Do not apply the patch yet and run the following command: `perl -cw Koha/Object.pm` It should print several warnings about redefined subroutines, meaning there is a circular dependency. 2. Apply the patch 3. Run `perl -cw Koha/Object.pm`. It should only say: "Koha/Object.pm syntax OK" 4. Run the following command: prove \ t/DateUtils.t \ t/Koha/DateTime/Format/RFC3339.t \ t/db_dependent/Koha/Object.t
Created attachment 163941 [details] [review] Bug 36432: Remove circular dependency from Koha::Object Koha::Object depends on Koha::DateUtils, which depends on C4::Context, which depends on Koha::Config::SysPref, which depends on... Koha::Object Apart from the circular dependency, the dependency on C4::Context alone is problematic as it loads a bunch of modules that are not needed at all in Koha::Object (YAML::XS and ZOOM for instance). As Koha::Object is used as a base for a lot of modules, we should take care to only load the minimum required. This patch moves some date parsing code to specific modules: - Koha::DateTime::Format::RFC3339 - Koha::DateTime::Format::SQL and it uses them in Koha::Object and Koha::DateUtils where it is possible. Test plan: 1. Do not apply the patch yet and run the following command: `perl -cw Koha/Object.pm` It should print several warnings about redefined subroutines, meaning there is a circular dependency. 2. Apply the patch 3. Run `perl -cw Koha/Object.pm`. It should only say: "Koha/Object.pm syntax OK" 4. Run the following command: prove \ t/DateUtils.t \ t/Koha/DateTime/Format/RFC3339.t \ t/db_dependent/Koha/Object.t
Created attachment 163942 [details] [review] Bug 36432: Remove circular dependency from Koha::Object Koha::Object depends on Koha::DateUtils, which depends on C4::Context, which depends on Koha::Config::SysPref, which depends on... Koha::Object Apart from the circular dependency, the dependency on C4::Context alone is problematic as it loads a bunch of modules that are not needed at all in Koha::Object (YAML::XS and ZOOM for instance). As Koha::Object is used as a base for a lot of modules, we should take care to only load the minimum required. This patch moves some date parsing code to specific modules: - Koha::DateTime::Format::RFC3339 - Koha::DateTime::Format::SQL and it uses them in Koha::Object and Koha::DateUtils where it is possible. Test plan: 1. Do not apply the patch yet and run the following command: `perl -cw Koha/Object.pm` It should print several warnings about redefined subroutines, meaning there is a circular dependency. 2. Apply the patch 3. Run `perl -cw Koha/Object.pm`. It should only say: "Koha/Object.pm syntax OK" 4. Run the following command: prove \ t/DateUtils.t \ t/Koha/DateTime/Format/RFC3339.t \ t/db_dependent/Koha/Object.t
Created attachment 163971 [details] [review] Bug 36432: Remove circular dependency from Koha::Object Koha::Object depends on Koha::DateUtils, which depends on C4::Context, which depends on Koha::Config::SysPref, which depends on... Koha::Object Apart from the circular dependency, the dependency on C4::Context alone is problematic as it loads a bunch of modules that are not needed at all in Koha::Object (YAML::XS and ZOOM for instance). As Koha::Object is used as a base for a lot of modules, we should take care to only load the minimum required. This patch moves some date parsing code to specific modules: - Koha::DateTime::Format::RFC3339 - Koha::DateTime::Format::SQL and it uses them in Koha::Object and Koha::DateUtils where it is possible. Test plan: 1. Do not apply the patch yet and run the following command: `perl -cw Koha/Object.pm` It should print several warnings about redefined subroutines, meaning there is a circular dependency. 2. Apply the patch 3. Run `perl -cw Koha/Object.pm`. It should only say: "Koha/Object.pm syntax OK" 4. Run the following command: prove \ t/DateUtils.t \ t/Koha/DateTime/Format/RFC3339.t \ t/db_dependent/Koha/Object.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
It works :) --- To spare a QA roundtrip here is the next step: FAIL Koha/DateTime/Format/RFC3339.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 5) FAIL Koha/DateTime/Format/SQL.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 2) WARN t/Koha/DateTime/Format/RFC3339.t WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 23)
Created attachment 163975 [details] [review] Bug 36432: Remove circular dependency from Koha::Object Koha::Object depends on Koha::DateUtils, which depends on C4::Context, which depends on Koha::Config::SysPref, which depends on... Koha::Object Apart from the circular dependency, the dependency on C4::Context alone is problematic as it loads a bunch of modules that are not needed at all in Koha::Object (YAML::XS and ZOOM for instance). As Koha::Object is used as a base for a lot of modules, we should take care to only load the minimum required. This patch moves some date parsing code to specific modules: - Koha::DateTime::Format::RFC3339 - Koha::DateTime::Format::SQL and it uses them in Koha::Object and Koha::DateUtils where it is possible. Test plan: 1. Do not apply the patch yet and run the following command: `perl -cw Koha/Object.pm` It should print several warnings about redefined subroutines, meaning there is a circular dependency. 2. Apply the patch 3. Run `perl -cw Koha/Object.pm`. It should only say: "Koha/Object.pm syntax OK" 4. Run the following command: prove \ t/DateUtils.t \ t/Koha/DateTime/Format/RFC3339.t \ t/db_dependent/Koha/Object.t
(In reply to Victor Grousset/tuxayo from comment #5) > To spare a QA roundtrip here is the next step: Thanks. I attached a new patch that should pass all QA tests
Created attachment 164056 [details] [review] Bug 36432: Remove circular dependency from Koha::Object Koha::Object depends on Koha::DateUtils, which depends on C4::Context, which depends on Koha::Config::SysPref, which depends on... Koha::Object Apart from the circular dependency, the dependency on C4::Context alone is problematic as it loads a bunch of modules that are not needed at all in Koha::Object (YAML::XS and ZOOM for instance). As Koha::Object is used as a base for a lot of modules, we should take care to only load the minimum required. This patch moves some date parsing code to specific modules: - Koha::DateTime::Format::RFC3339 - Koha::DateTime::Format::SQL and it uses them in Koha::Object and Koha::DateUtils where it is possible. Test plan: 1. Do not apply the patch yet and run the following command: `perl -cw Koha/Object.pm` It should print several warnings about redefined subroutines, meaning there is a circular dependency. 2. Apply the patch 3. Run `perl -cw Koha/Object.pm`. It should only say: "Koha/Object.pm syntax OK" 4. Run the following command: prove \ t/DateUtils.t \ t/Koha/DateTime/Format/RFC3339.t \ t/db_dependent/Koha/Object.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
thanks :) ---- Added back signoff lost in the process.
Created attachment 164210 [details] [review] Bug 36432: Remove circular dependency from Koha::Object Koha::Object depends on Koha::DateUtils, which depends on C4::Context, which depends on Koha::Config::SysPref, which depends on... Koha::Object Apart from the circular dependency, the dependency on C4::Context alone is problematic as it loads a bunch of modules that are not needed at all in Koha::Object (YAML::XS and ZOOM for instance). As Koha::Object is used as a base for a lot of modules, we should take care to only load the minimum required. This patch moves some date parsing code to specific modules: - Koha::DateTime::Format::RFC3339 - Koha::DateTime::Format::SQL and it uses them in Koha::Object and Koha::DateUtils where it is possible. Test plan: 1. Do not apply the patch yet and run the following command: `perl -cw Koha/Object.pm` It should print several warnings about redefined subroutines, meaning there is a circular dependency. 2. Apply the patch 3. Run `perl -cw Koha/Object.pm`. It should only say: "Koha/Object.pm syntax OK" 4. Run the following command: prove \ t/DateUtils.t \ t/Koha/DateTime/Format/RFC3339.t \ t/db_dependent/Koha/Object.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Thanks for testing! I added some unit tests for Koha::DateTime::Format::SQL in bug 36486
Pushed for 24.05! Well done everyone, thank you!
Pushed to 23.11.x for 23.11.06
Backported to 23.05.x for upcoming 23.05.12