Fix Zonemaster::Engine::Translator’s instance() method#1347
Merged
marc-vanderwal merged 2 commits intozonemaster:developfrom Jun 3, 2024
Merged
Fix Zonemaster::Engine::Translator’s instance() method#1347marc-vanderwal merged 2 commits intozonemaster:developfrom
marc-vanderwal merged 2 commits intozonemaster:developfrom
Conversation
A programming error in Zonemaster::Engine::Translator’s instance() method caused the global variable $instance to be immediately undefined right after the initialize() function set it to an instance of a Translator object, if that singleton object hadn’t been previously instanciated. The documentation for Zonemaster::Engine::Translator’s initialize() method was also incorrect: this function takes a raw list of arguments, not a hash reference. Do not use the new() method in the corresponding unit tests anymore, because that method is deprecated.
Contributor
Author
The documentation for Zonemaster::Engine::Translator still referred to Moose in one place.
b8895fe to
c65eb5b
Compare
Contributor
Author
|
I’m keeping this PR as a draft for now, because I’m considering also deprecating the |
Contributor
Author
That’s better done in another PR. |
mattias-p
approved these changes
May 28, 2024
matsduf
approved these changes
Jun 3, 2024
marc-vanderwal
added a commit
to marc-vanderwal/zonemaster-backend
that referenced
this pull request
Jun 3, 2024
Pull request zonemaster-engine#1347 (zonemaster/zonemaster-engine#1347) has been merged, which fixes Zonemaster::Backend::Translator->instance(). So we can remove one use of the deprecated Zonemaster::Backend::Translator->new() in the unit tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This PR fixes a bug in Zonemaster::Engine::Translator that causes problems in Zonemaster::Backend::Translator (in Backend, not Engine), which inherits from Zonemaster::Engine::Translator.
Context
While adding unit tests in Zonemaster::Backend for its Translator module, I noticed that the
instance()method in Zonemaster::Engine::Translator always returnedundefinstead of a Translator object.Changes
instance()method which prevented it from working as documented.instance()instead ofnew(), to exercise the code.How to test this PR
Run the test suite in Zonemaster::Engine. Expect all unit tests to pass.