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

Chore: use ecmarkup formatter #2623

Open
justingrant opened this issue Jun 29, 2023 · 6 comments
Open

Chore: use ecmarkup formatter #2623

justingrant opened this issue Jun 29, 2023 · 6 comments
Labels
editorial tools Tools for managing this repo
Milestone

Comments

@justingrant
Copy link
Collaborator

From tc39/ecmarkup#536 (comment):

Handling of whitespace is generally managed by the formatter, not the build job, and indeed if you run the formatter on this input it will fix it for you.

As we get closer to Stage 4, it may be helpful to transform our spec text into the standard format used in ECMA-262, so that diffs will be cleaner.

That said, it will create a huge one-time diff when it's run for the first time, so we should pick a time to start formatting when there are no huge PRs pending, in order to reduce any rebase pain for in-progress PRs.

@justingrant justingrant added editorial tools Tools for managing this repo labels Jun 29, 2023
@bakkot
Copy link
Contributor

bakkot commented Jun 29, 2023

Re: rebase pain, rebases of this kind can be completely automated, so shouldn't be too bad.

That said, right now Temporal uses multi-line records in source, which ecmarkup doesn't explicitly support, so you may want to hold off on running the formatter until it does (or stop using them, or something).

@justingrant justingrant added this to the Stage 4 milestone Jun 29, 2023
@ptomato
Copy link
Collaborator

ptomato commented Jul 11, 2023

If I understand what you mean by "multi-line records" then we already use them inconsistently, so it's probably not too bad to drop them.

@justingrant
Copy link
Collaborator Author

What's a multi-line record?

@bakkot
Copy link
Contributor

bakkot commented Jul 11, 2023

Writing a record on multiple lines, like this.

@gibson042
Copy link
Collaborator

Related: tc39/ecmarkup#513

@justingrant
Copy link
Collaborator Author

Meeting 2023-08-17: We'll make this change in CI to add the formatter, but only after the big in-flight PRs have been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial tools Tools for managing this repo
Projects
None yet
Development

No branches or pull requests

4 participants