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

feat(grit): complete formatting of all nodes #4520

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

ematipico
Copy link
Member

Summary

This PR completes the formatting of grit, using straightforward logic. We will improve the formatting as we go, and as we identify possible code that could be improved

Comments aren't appropriately handled yet in the sense that we don't have specific logic about their placement in certain conditions.

I also renamed the node GritCurlyPredicateList to GritPredicateCurly. The node wasn't a list, and I renamed it following the internal convention.

Test Plan

I updated the snapshots. I couldn't find the code for the newly formatted nodes.

@github-actions github-actions bot added A-Parser Area: parser A-Formatter Area: formatter A-Tooling Area: internal tools L-Grit Language: GritQL labels Nov 12, 2024
@ematipico ematipico requested review from a team November 12, 2024 17:36
Copy link
Contributor

Parser conformance results on

js/262

Test result main count This PR count Difference
Total 48591 48591 0
Passed 47399 47399 0
Failed 1192 1192 0
Panics 0 0 0
Coverage 97.55% 97.55% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6586 6586 0
Passed 2208 2208 0
Failed 4378 4378 0
Panics 0 0 0
Coverage 33.53% 33.53% 0.00%

ts/babel

Test result main count This PR count Difference
Total 680 680 0
Passed 608 608 0
Failed 72 72 0
Panics 0 0 0
Coverage 89.41% 89.41% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 18615 18615 0
Passed 14270 14270 0
Failed 4345 4345 0
Panics 0 0 0
Coverage 76.66% 76.66% 0.00%

Copy link

codspeed-hq bot commented Nov 12, 2024

CodSpeed Performance Report

Merging #4520 will not alter performance

Comparing feat/grit-formatting-part-3 (143520d) with main (929e86d)

Summary

✅ 99 untouched benchmarks

Copy link
Contributor

@arendjr arendjr left a comment

Choose a reason for hiding this comment

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

Really nice!

Maybe you can do a small announcement on it as well when you merge ;)

@dyc3
Copy link
Contributor

dyc3 commented Nov 12, 2024

I would make sure it's available in the playground before making an announcement so people can try it out.

@ematipico
Copy link
Member Author

I would make sure it's available in the playground before making an announcement so people can try it out.

I still need to understand why it currently fails 🤔 weird

@ematipico
Copy link
Member Author

Really nice!

Maybe you can do a small announcement on it as well when you merge ;)

Not yet, there are still things missing :)

@ematipico ematipico merged commit 9611497 into main Nov 13, 2024
13 checks passed
@ematipico ematipico deleted the feat/grit-formatting-part-3 branch November 13, 2024 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Formatter Area: formatter A-Parser Area: parser A-Tooling Area: internal tools L-Grit Language: GritQL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants