-
Notifications
You must be signed in to change notification settings - Fork 295
Add Augmented Latin IDN table to IDN enums #2914
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
Conversation
weiminyu
left a comment
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.
@weiminyu reviewed 7 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @CydeWeys).
weiminyu
left a comment
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.
@weiminyu made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @CydeWeys).
core/src/test/resources/google/registry/beam/rde/reducer_rde.xml line 42 at r1 (raw file):
<rdeHeader:count uri="urn:ietf:params:xml:ns:rdeIDN-1.0">4</rdeHeader:count> </rdeHeader:header>
Looks like this file needs more changes.
RdePipelineTest is still failing.
186c271 to
0499d8b
Compare
This was added in google#2884 , but now as of this PR it can actually be configured and used on a TLD.
0499d8b to
29c9ce0
Compare
CydeWeys
left a comment
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.
@CydeWeys made 1 comment.
Reviewable status: 6 of 8 files reviewed, 1 unresolved discussion (waiting on @weiminyu).
core/src/test/resources/google/registry/beam/rde/reducer_rde.xml line 42 at r1 (raw file):
Previously, weiminyu (Weimin Yu) wrote…
Looks like this file needs more changes.
RdePipelineTest is still failing.
PTAL, fixed.
weiminyu
left a comment
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.
@weiminyu reviewed 2 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @CydeWeys).
weiminyu
left a comment
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.
@weiminyu made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @CydeWeys).
This was added in #2884 , but now as of this PR it can actually be configured and used on a TLD.
This change is