Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix option --[no-]json-translate #337

Merged
4 commits merged into from Jun 7, 2023
Merged

Fix option --[no-]json-translate #337

4 commits merged into from Jun 7, 2023

Conversation

ghost
Copy link

@ghost ghost commented May 31, 2023

Purpose

This PR fixes 2 small error/bugs discovered during v2023.1 release testing.

Context

#308 #309
v2023.1 release testing comment #308 (comment)
v2023.1 release testing comment #309 (comment)

Changes

  • fix ineffective --no-json-translate option
  • show deprectation warning with option alias --[no-]json_translate

How to test this PR

  • Check that --[no-]json_translate option gives a deprecation warning.
  • Check that --no-json-translate does not output the message key.

@ghost ghost added the T-Bug Type: Bug in software or error in test case description label May 31, 2023
@ghost ghost added this to the v2023.1 milestone May 31, 2023
@ghost ghost requested review from mattias-p, matsduf, hannaeko, tgreenx and marc-vanderwal May 31, 2023 14:28
@tgreenx tgreenx added the V-Patch Versioning: The change gives an update of patch in version. label May 31, 2023
tgreenx
tgreenx previously approved these changes May 31, 2023
@ghost
Copy link
Author

ghost commented May 31, 2023

Finally I went with a more complete solution were --[no-]raw takes precedence over --[no-]json-translate.

Alexandre Pion added 3 commits June 1, 2023 11:04
The default value of the `raw` property is removed and the `raw`
property value is set later. Nice suggestion from @mattias-p.
Here are the different possibilities for the assignment of the "raw"
property:

  ∅  means that the option is not passed and the property undefined
  0  means that the option is passed in its negated form (property = 0)
  1  means that the option is passed in its positive form (property = 1)

                                +---------------+
                                |      raw      |
                                | ------------- |
                                |  ∅    0    1  |
        +-----------------------+---------------+
        |                 |  ∅  |  0    0    1  |
        |  json_translate |  0  |  1    0    1  |
        |                 |  1  |  0    0    1  |
        +-----------------------+---------------+
           assigned value to the "raw" property
@ghost
Copy link
Author

ghost commented Jun 1, 2023

I've refactored the PR. There was a small glitch with your suggestion @mattias-p. If there is no option passed, the output should be translated (raw == 0). I've added some logic to handle this case as well.

@matsduf matsduf added the P-High Priority: Issue to be solved before other label Jun 7, 2023
@matsduf matsduf requested review from mattias-p and tgreenx June 7, 2023 13:05
Copy link
Contributor

@tgreenx tgreenx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works as expected.

@ghost ghost merged commit 6934e37 into zonemaster:develop Jun 7, 2023
@ghost ghost deleted the fix-json-translate branch June 7, 2023 13:44
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-High Priority: Issue to be solved before other T-Bug Type: Bug in software or error in test case description V-Patch Versioning: The change gives an update of patch in version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants