Description
Julian Maurice
2024-03-26 15:36:36 UTC
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 |