Rename more message args #799

mattias-p · 2020-10-05T15:12:10Z

Renames a bunch more name server related message arguments. No new argument names are introduced, except suffixes are appended to previously documented names, in order to provide disambiguation and context.

One unused message is also removed. In one case unused arguments are removed from the construction of a message.

This is a step towards solving #713.

matsduf · 2020-10-05T18:42:37Z

lib/Zonemaster/Engine/Test/Consistency.pm

    },
    IN_BAILIWICK_ADDR_MISMATCH => sub {
        __x    # CONSISTENCY:IN_BAILIWICK_ADDR_MISMATCH
          'In-bailiwick name server listed at parent has a mismatch between glue data at parent '
-          . '({parent_addresses}) and any equivalent address record in child zone ({zone_addresses}).',
+          . '({ns_list_parent}) and any equivalent address record in child zone ({ns_list_zone}).',


The new names "ns_list_parent" and "ns_list_zone" are not defined in "docs/logentry_args.md", which should therefore be updated and included in this PR.

I think "parent_ns_list" makes more sense. It could be possible that a single "ns" is referred to in a message (I mean single, not a list with one member) and then "parent_ns" would match better. Secondly, it is rather a list of parent ns, i.e. "parent" and "ns" are more connected.

Same with "zone".

The way I see this is that ns_list_parent (or parent_ns_list) is a variant of ns_list. I don't it's necessary to list all variant names.

If we use suffixes for variants instead of prefixes, it's easier to find the canonical name in the list.

I don't it's necessary to list all variant names.

We really should.

If we use suffixes for variants instead of prefixes, it's easier to find the canonical name in the list.

Possibly, but then "ns_parent_list" makes more sense.

matsduf · 2020-10-05T18:51:40Z

lib/Zonemaster/Engine/Test/Consistency.pm

@@ -204,11 +200,11 @@ Readonly my %TAG_DESCRIPTIONS => (
    },
    NS_SET => sub {
        __x    # CONSISTENCY:NS_SET
-          'Saw NS set ({nsset}) on following nameserver set : {servers}.', @_;
+          'Saw NS set ({nsname_list_response}) on following nameserver set : {ns_list_servers}.', @_;


Not defined in "docs/logentry_args.md".

I think the second one ({servers}) should be "ns_list". It is just a list of NS/IP pairs that have been visited/investigate, isn't it?

Why cannot the first one be "nsname_list"?

We could have nsname_list and ns_list here - there's no technical need for disambiguation. I only added the suffixes for extra clarity, but we could also go for shorter names and leave it to the msgid to provide context.

It is not a question of shorter names. We should as few defined argument names as possible to make it easier for the translator. In this example we have one list of NS names and one list of NS/IP pairs. Why should they be called something else in this msgid than in others?

matsduf · 2020-10-05T18:53:21Z

lib/Zonemaster/Engine/Test/Consistency.pm

    },
    ONE_NS_SET => sub {
        __x    # CONSISTENCY:ONE_NS_SET
-          "A single NS set was found ({nsset}).", @_;
+          "A single NS set was found ({nsname_list}).", @_;


Compare with NS_SET above.

I don't get this. Can you elaborate?

I am sorry, I was unclear. The msgid's for NS_SET and ONS_NS_SET, respectively, both have an argument for a set of NS names. In both cases it should be changed from "nsset" to "nsname_list".

matsduf · 2020-10-05T18:55:13Z

lib/Zonemaster/Engine/Test/Consistency.pm

-          . 'the glue at the parent ({parent_addresses}) and any equivalent address record found '
-          . 'in authoritative zone ({zone_addresses}).', @_;
+          . 'the glue at the parent ({ns_list_parent}) and any equivalent address record found '
+          . 'in authoritative zone ({ns_list_zone}).', @_;
    },


See comment above.

matsduf · 2020-10-05T18:55:39Z

lib/Zonemaster/Engine/Test/Consistency.pm

    },
    SOA_RNAME => sub {
        __x    # CONSISTENCY:SOA_RNAME
-          "Found SOA rname {rname} on following nameserver set : {servers}.", @_;
+          "Found SOA rname {rname} on following nameserver set : {ns_list}.", @_;
    },


~~Yes, compare with NS_SET above.~~

I don't get this. Can you elaborate?

(Comment removed.)

mattias-p · 2020-10-06T10:16:18Z

I'll update the documentation to clarify the usage of suffixes.

matsduf · 2020-10-06T11:56:04Z

docs/logentry_args.md

+for disambiguation and/or clarity.
+Suffixes must be in snake_case, they must start with an underscore, and it must
+be unambiguous from the table where the base name ends and where the suffix
+begins.


You have changed this document from a list of defined argument names to something more complex that includes something called "base name" and some kind of rules for suffixing, which means that a name has some kind of morphology or syntax.

I prefer if this is a list defined argument names, i.e. names without formal internal structure. At least as a start.

If you go the second way, some terms must be defined and the rules must be specified. Still, the suffixes or prefixes must be listed and defined. It must be possible, as a translator, to go into the list and find the argument name and figure out what kind of data it is.

matsduf · 2020-10-06T11:57:06Z

lib/Zonemaster/Engine/Test/Consistency.pm

    },
    IN_BAILIWICK_ADDR_MISMATCH => sub {
        __x    # CONSISTENCY:IN_BAILIWICK_ADDR_MISMATCH
          'In-bailiwick name server listed at parent has a mismatch between glue data at parent '
-          . '({parent_addresses}) and any equivalent address record in child zone ({zone_addresses}).',
+          . '({ns_list_parent}) and any equivalent address record in child zone ({ns_list_zone}).',


I don't it's necessary to list all variant names.

We really should.

If we use suffixes for variants instead of prefixes, it's easier to find the canonical name in the list.

Possibly, but then "ns_parent_list" makes more sense.

matsduf · 2020-10-06T11:59:40Z

lib/Zonemaster/Engine/Test/Consistency.pm

@@ -204,11 +200,11 @@ Readonly my %TAG_DESCRIPTIONS => (
    },
    NS_SET => sub {
        __x    # CONSISTENCY:NS_SET
-          'Saw NS set ({nsset}) on following nameserver set : {servers}.', @_;
+          'Saw NS set ({nsname_list_response}) on following nameserver set : {ns_list_servers}.', @_;


It is not a question of shorter names. We should as few defined argument names as possible to make it easier for the translator. In this example we have one list of NS names and one list of NS/IP pairs. Why should they be called something else in this msgid than in others?

mattias-p · 2020-10-08T14:33:37Z

@matsduf Can you review again?

matsduf

I think we should have as few argument names as possible. They should focus on the kind of data. That is what the translators need.

matsduf · 2020-10-08T15:46:21Z

lib/Zonemaster/Engine/Test/Consistency.pm

    },
    ONE_NS_SET => sub {
        __x    # CONSISTENCY:ONE_NS_SET
-          "A single NS set was found ({nsset}).", @_;
+          "A single NS set was found ({nsname_list}).", @_;


I am sorry, I was unclear. The msgid's for NS_SET and ONS_NS_SET, respectively, both have an argument for a set of NS names. In both cases it should be changed from "nsset" to "nsname_list".

matsduf · 2020-10-08T15:53:06Z

lib/Zonemaster/Engine/Test/Consistency.pm

    },
    SOA_RNAME => sub {
        __x    # CONSISTENCY:SOA_RNAME
-          "Found SOA rname {rname} on following nameserver set : {servers}.", @_;
+          "Found SOA rname {rname} on following nameserver set : {ns_list}.", @_;
    },


(Comment removed.)

mattias-p · 2020-10-09T07:44:49Z

@matsduf Can you approve?

mattias-p added 9 commits October 2, 2020 18:52

Fix message arg name mismatch

fde74e2

Rename message arg: addrs

b64d7f3

Rename message arg: servers

e4a87f1

Rename message arg: glue

ee9076a

Rename message args: parent_addresses, zone_addresses

00e719f

Rename message arg: addresses

00a844e

Refactor

6820639

Rename message arg: nsset

bda4006

Rename message arg: names

02189bd

mattias-p requested review from matsduf, vlevigneron and sandoche2k October 5, 2020 16:18

matsduf requested changes Oct 5, 2020

View reviewed changes

matsduf added this to the v2020.1 milestone Oct 5, 2020

matsduf added the A-TestCase Area: Test case specification or implementation of test case label Oct 5, 2020

Clarify usage of suffixes

63228ac

matsduf requested changes Oct 6, 2020

View reviewed changes

Revert suffixed argument names

0a2c1a6

Rename nssset argument

bf127cf

matsduf requested changes Oct 9, 2020

View reviewed changes

matsduf approved these changes Oct 9, 2020

View reviewed changes

sandoche2k approved these changes Oct 9, 2020

View reviewed changes

mattias-p merged commit cbd4889 into zonemaster:develop Oct 12, 2020

mattias-p deleted the 713-msg-args branch January 7, 2021 11:48

matsduf mentioned this pull request Feb 8, 2021

Standardized and defined arguments in msgid #713

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename more message args #799

Rename more message args #799

mattias-p commented Oct 5, 2020 •

edited

Loading

matsduf Oct 5, 2020

mattias-p Oct 6, 2020

matsduf Oct 6, 2020 •

edited

Loading

matsduf Oct 5, 2020

mattias-p Oct 6, 2020

matsduf Oct 6, 2020

matsduf Oct 5, 2020

mattias-p Oct 8, 2020

matsduf Oct 8, 2020

matsduf Oct 5, 2020

matsduf Oct 5, 2020 •

edited

Loading

mattias-p Oct 8, 2020

matsduf Oct 8, 2020

mattias-p commented Oct 6, 2020

matsduf Oct 6, 2020

matsduf Oct 6, 2020 •

edited

Loading

matsduf Oct 6, 2020

mattias-p commented Oct 8, 2020

matsduf left a comment

matsduf Oct 8, 2020

matsduf Oct 8, 2020

mattias-p commented Oct 9, 2020

Rename more message args #799

Rename more message args #799

Conversation

mattias-p commented Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matsduf Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matsduf Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattias-p commented Oct 6, 2020

Choose a reason for hiding this comment

matsduf Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattias-p commented Oct 8, 2020

matsduf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattias-p commented Oct 9, 2020

mattias-p commented Oct 5, 2020 •

edited

Loading

matsduf Oct 6, 2020 •

edited

Loading

matsduf Oct 5, 2020 •

edited

Loading

matsduf Oct 6, 2020 •

edited

Loading