Skip to content

Reference ECMAScript 11.0 specifically (#821) #965

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

Merged
merged 8 commits into from
Nov 4, 2020

Conversation

Relequestual
Copy link
Member

@Relequestual Relequestual commented Aug 10, 2020

Fixes #821
Reference specific version of emca262.

Copy link
Member

@ssilverman ssilverman left a comment

Choose a reason for hiding this comment

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

I’d like to encourage a better PR title.

@awwright awwright changed the title Fixes 821 Reference ECMAScript 5.1 specifically (#821) Sep 4, 2020
@awwright
Copy link
Member

awwright commented Sep 4, 2020

@ssilverman Good suggestion
@Relequestual I also added a # to your PR description to make it a link

@Relequestual Relequestual changed the title Reference ECMAScript 5.1 specifically (#821) Reference ECMAScript 10.0 specifically (#821) Sep 30, 2020
Copy link
Member

@awwright awwright left a comment

Choose a reason for hiding this comment

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

I'm not sure if this is in the scope of this PR, but as I noted in #821, we also need to be specific about the flags the RegExp is built with: The u flag changes how backslash escapes work, but is necessary for Unicode support.

@Relequestual
Copy link
Member Author

I'm not sure if this is in the scope of this PR, but as I noted in #821, we also need to be specific about the flags the RegExp is built with: The u flag changes how backslash escapes work, but is necessary for Unicode support.

Sure, makes sense to me. I'll make a change.

@Relequestual
Copy link
Member Author

@awwright I've pushed a new commit which notes that regular expression SHOULD be built with support for unicode as per defined in ECMA 262.

If you approve of this change, we can merge this!

@Relequestual Relequestual marked this pull request as ready for review October 14, 2020 10:34
@Relequestual
Copy link
Member Author

The above commit mess is brought to you by "Using caps in your branch names!"
Never do that =]

Copy link
Member

@ssilverman ssilverman left a comment

Choose a reason for hiding this comment

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

Change the PR title to reference "11th edition" instead of the 10th.

@karenetheridge
Copy link
Member

The commit message also references 10 (presumably a rebase and rewording will be done after all reviewing is complete?)

@Relequestual Relequestual changed the title Reference ECMAScript 10.0 specifically (#821) Reference ECMAScript 11.0 specifically (#821) Nov 3, 2020
@Relequestual
Copy link
Member Author

The commit message also references 10 (presumably a rebase and rewording will be done after all reviewing is complete?)

I guess so, but I don't usually. But OK.

@Relequestual
Copy link
Member Author

Given the unicode issue is now resolved. I'll consider this OK to merge once automated checks are complete.

@Relequestual Relequestual merged commit af23dbd into json-schema-org:master Nov 4, 2020
@Relequestual Relequestual deleted the fixes/821 branch November 4, 2020 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify which regular expression production rule from the ECMA specification is intended
6 participants