-
Notifications
You must be signed in to change notification settings - Fork 33
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
Define argument name "label" and "unicode_name" #1126
Define argument name "label" and "unicode_name" #1126
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beside a suggestion (not necessarily related to the PR), this looks ok.
@@ -91,7 +92,6 @@ defined *arguments*. | |||
|| DNS packet size| The size in octets of a DNS packets.| | |||
|| DNSKEY key length| The key length for a DNSKEY. The interpretation of this value various quite a bit with the algorithm. Be careful when using it for algorithms that aren't RSA-based.| | |||
|| DNSSEC delegation verification failure reason| A somewhat human-readable reason why the delegation step between the tested zone and its parent is not secure.| | |||
| dlabel (?) | Domain name label| A single label from a domain name.| | |||
| dlength (?) | Domain name label length| The length of a domain name label.| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the dlength
is used, have you considered moving it to the Defined arguments
table?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did consider that too, but I could not make up my mind if it should be "length" (since all lenght I know of is a positive integer) and defined as "Length of some entity" or "labellength"? If just "length" then it could be used in several cases.
In the same message ID there is also a "max". Should that be "max", "maxlength" or "maxlabellenght"?
There are several argument names to define.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know either, but if we concatenate several words, I'd suggest to stick to snake case as already done for other labels. Hence it could be "label_length".
@@ -70,7 +71,7 @@ and updated messages (*msgids* and *msgstr*). | |||
| rcode | An RCODE Name | An RCODE Name (not numeric code) from [DNS RCODEs]. | | |||
| rrtype | A Resource Record TYPE Name | A Resource Record TYPE Name (not numeric code) from [DNS RR TYPEs]. | | |||
| testcase | A Zonemaster test case, or `all` | A test case identifier. | | |||
|
|||
| unicode_name | Unicode name of a code point | The name is a string in ASCII only and in upper case, e.g. "LATIN SMALL LETTER A"| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition, the argument name "unicode_name" is defined (not yet in use in any code).
Do you have an example of how this argument would be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found it in zonemaster/zonemaster@8a14f99 (from zonemaster/zonemaster#942)
Purpose
Both "dlabel" and "label" are used as argument names, but none of them are defined. The former is used only once, the latter is clearer and used several times. With this PR "label" is defined in the documentation and "dlabel" is not used any more.
To make implementation consistent, "dlabel" is replaced by "label" in the affected Perl module and in the PO files.
In addition, the argument name "unicode_name" is defined (not yet in use in any code).
Context
In zonemaster/zonemaster#942 (comment) it was noted that undefined "dlabel" was used in that PR. Instead of defining that, it will be replaced by "label" that this PR defines.
Changes
How to test this PR
The main change is documentation change only. Review the change and compare with the implementation.
The other change affects the PO files. That can be tested by following the "Instructions for translators":
./update-po da.po
and for all other PO files.../util/check-msg-args da.po
and for all other PO files.