-
Notifications
You must be signed in to change notification settings - Fork 34
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
Separate functions to trim whitespace and normalize domain name #1316
base: develop
Are you sure you want to change the base?
Conversation
Also, fix resulting compilation errors and make iteration order deterministic.
* Better diagnostics. * More complete test for bad domain names.
@matsduf has expressed that he wants to update RequirementsAndNormalizationOfDomainNames.md before this is merged. |
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.
If trimming of leading and trailing white space is removed from Engine, then the function must be in GUI or in Backend. Do we have the function in GUI?
Another option would be that Engine offers two alternatives, trimming of leading and trailing white space enabled and trimming disabled, respectively. That would make it easy for a new client to choose.
Also, we have zmb
in Backend. Should that behave like GUI and trim? Or should that be like CLI and not trim? If zmb
should trim (to make it more GUI like) then trimming should be done in Backend.
@matsduf Regarding your comment above, as you remember, we discussed this topic at a work group meeting a while ago. I believe it was after your comment, but it seems we forgot to note our conclusions here. Instead of leaving (seemingly) unanswered questions hanging, it seems appropriate to summarize some clarifications along the lines of our discussion as I remember it.
@matsduf, @marc-vanderwal, @tgreenx, @MichaelTimbert do you guys agree with the above? |
@mattias-p, please see zonemaster/zonemaster#1260. |
@mattias-p, you do not trimming to be part of the normalization specification (you write "test case specification" but I think you mean the normalization document), but you want the normalization to happen in GUI. Where do you want that to be specified or do you prefer it to be unspecified. I am not sure that I agree with your bullets, but I will not totally object. A requirement is that operations happening are documented and specified. |
Purpose
This PR separates the normalization of what's logically part of domain names from the white space stripping that is also specified in RequirementsAndNormalizationOfDomainNames.md to make it possible to apply them individually.
Context
This PR fixes item 3 in zonemaster/zonemaster-cli#364.
zonemaster/zonemaster-backend#1143 should be merged together with this one.
This is technically a major change because normalize_name() is part of the public API. However, even though we've included an implementation in prior releases, we've never actually called it (except from unit tests).
Changes
How to test this PR
Zonemaster CLI should present the following behaviors: