Skip to content

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

Merged
4 commits merged intodevelopfrom
unknown repository
Jun 7, 2023
Merged

Fix option --[no-]json-translate#337
4 commits merged intodevelopfrom
unknown repository

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 hannaeko, marc-vanderwal, matsduf, mattias-p and tgreenx 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