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

Compact parser fixture loc info #11322

Merged
merged 2 commits into from Mar 24, 2020

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Mar 23, 2020

Q                       A
License MIT

Another MX (Maintinaer Experience) PR.

When working on babel parser, the fixture is typically too long to skim. One of obstacles are the loc info occupies many lines. This PR compacts the fixtures a bit so now it should be easier to read the test fixtures.

For example, see if you can read this fixture now: https://github.com/babel/babel/blob/bd783c4543d74ffa8f2a248ea449eda64d4fdf77/packages/babel-parser/test/fixtures/comments/basic/array-pattern-trailing-comma/output.json

Please review the first commit only as the whole diff turns out too large for GitHub.

The idea is stolen from the Flow repo.

@JLHwung JLHwung added the PR: Internal 🏠 A type of pull request used for our changelog categories label Mar 23, 2020
@JLHwung JLHwung force-pushed the compact-parser-fixture-loc-info branch from bd783c4 to 78af985 Compare March 23, 2020 21:31
Copy link
Member

@kaicataldo kaicataldo left a comment

Choose a reason for hiding this comment

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

This is a very cool idea 👍

nicolo-ribaudo
nicolo-ribaudo previously approved these changes Mar 23, 2020
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

This is dark magic

@nicolo-ribaudo nicolo-ribaudo dismissed their stale review March 23, 2020 21:45

Oh the CI failures are related.

@JLHwung JLHwung force-pushed the compact-parser-fixture-loc-info branch from 78af985 to a93d80b Compare March 23, 2020 22:49
@JLHwung
Copy link
Contributor Author

JLHwung commented Mar 23, 2020

@nicolo-ribaudo CI is green now. 😄

Copy link
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

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

🔥

@JLHwung JLHwung merged commit dc7c564 into babel:master Mar 24, 2020
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 23, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants