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.

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

Editorial: remove unnecessary assertion #2296

Merged
merged 1 commit into from
Feb 12, 2021
Merged

Conversation

ljharb
Copy link
Member

@ljharb ljharb commented Feb 6, 2021

Both S and R are Strings, and both are asserted in the prose, so either both or neither should be asserted in the steps. I chose neither.

@ljharb ljharb requested review from syg, michaelficarra, bakkot and a team February 6, 2021 04:16
@jmdyck
Copy link
Collaborator

jmdyck commented Feb 6, 2021

That's consistent with this comment in PR #545, which proposes that:

  • a structured header (currently reflected by a standardized preamble) would give parameter invariants,
  • ecmarkup would render these as Assert steps,
  • and so any equivalent assertions that are already present should be removed.

@ljharb ljharb added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Feb 11, 2021
Both S and R are Strings, so either both or neither should be asserted in the steps. I chose neither.
@ljharb ljharb merged commit 97c2b87 into tc39:master Feb 12, 2021
@ljharb ljharb deleted the extra-assertion branch February 12, 2021 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants