Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make the IRI advice list be positive form advice #132

Merged
merged 2 commits into from
Jan 23, 2025
Merged

Make the IRI advice list be positive form advice #132

merged 2 commits into from
Jan 23, 2025

Conversation

afs
Copy link
Contributor

@afs afs commented Jan 16, 2025 •
edited by pr-preview bot
Loading

The IRI advice list had some negative forms which could be misread because other points were positive form.

Closes #63
(updates the punycode reference).

Preview | Diff

@afs afs requested review from gkellogg and hartig January 16, 2025 19:26
This was referenced Jan 16, 2025
Merged
Merged
TallTed
TallTed suggested changes Jan 22, 2025
View reviewed changes
Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth putting all the preferred examples into parentheses, or removing all those parentheses, and making the punctuation the same. (See lines 653-660)

spec/index.html Outdated Show resolved Hide resolved
@afs afs force-pushed the iri-advice branch 2 times, most recently from f5c4bb0 to 6882275 Compare January 22, 2025 15:30
@afs
Copy link
Contributor Author

afs commented Jan 22, 2025

It might be worth putting all the preferred examples into parentheses, or removing all those parentheses, and making the punctuation the same. (See lines 653-660)

Done.

@TallTed
Copy link
Member

TallTed commented Jan 23, 2025

Typo of perferred remains, though it is marked "resolved".

Please don't mark anything "resolved" unless you've clicked the COMMIT button and/or I've confirmed that you've made changes that resolve my concern.

@afs afs self-assigned this Jan 23, 2025
@afs
Copy link
Contributor Author

afs commented Jan 23, 2025

Fixed.

@afs afs merged commit 07a3d14 into main Jan 23, 2025
2 checks passed
@afs afs deleted the iri-advice branch January 23, 2025 18:17
aphillips
aphillips reviewed Feb 7, 2025
View reviewed changes
Copy link
Contributor

@aphillips aphillips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, @xfq was on vacation when this PR was being reviewed. I made some comments after the fact and will call them out in #63.

spec/index.html Show resolved Hide resolved
spec/index.html Show resolved Hide resolved
spec/index.html Show resolved Hide resolved
This was referenced Feb 7, 2025
Open
Open
@afs afs mentioned this pull request Feb 7, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gkellogg gkellogg gkellogg approved these changes

@aphillips aphillips aphillips left review comments

@TallTed TallTed TallTed requested changes

@pchampin pchampin pchampin approved these changes

@hartig hartig hartig approved these changes

Assignees

@afs afs

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Reference for punycode-encoding of IDN
6 participants
@afs @TallTed @gkellogg @aphillips @pchampin @hartig