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

Use our shared prettier config #1524

Merged
merged 1 commit into from
Mar 3, 2023
Merged

Use our shared prettier config #1524

merged 1 commit into from
Mar 3, 2023

Conversation

ahuth
Copy link
Contributor

@ahuth ahuth commented Mar 3, 2023

Summary:

Not sure if this was intended from #1457 or not, but it turns out we aren't actually using our shared prettier config. More deets inline.

This PR

  • Fixes that so we're using prettier-config-edu.
  • Introduces an override to avoid a large diff. We may want to remove this override at some point, though.

Test Plan:

  • Wrote automated tests
  • CI tests / new tests are not applicable
  • Manually tested my changes, but I want to keep the details secret
  • Manually tested my changes, and here are the details:
    • Introduced prettier violations
    • Observed prettier complaining/fixing

prettier-config-edu is not a prettier plugin, so passing it to `plugins`
isn't doing anything.

Instead, it's just an object that we can export (or spread) from our
config.
@codecov
Copy link

codecov bot commented Mar 3, 2023

Codecov Report

Merging #1524 (c98ea51) into main (bc21f85) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1524   +/-   ##
=======================================
  Coverage   91.97%   91.97%           
=======================================
  Files         284      284           
  Lines        4300     4300           
  Branches      793      793           
=======================================
  Hits         3955     3955           
  Misses        320      320           
  Partials       25       25           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

{
"trailingComma": "all",
"singleQuote": true,
"plugins": ["@chanzuckerberg/prettier-config-edu"]
Copy link
Contributor Author

@ahuth ahuth Mar 3, 2023

Choose a reason for hiding this comment

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

prettier-config-edu isn't a plugin, so adding it here wasn't actually using it.

Or at least wasn't using it for the rules config. But maybe was somehow configuring the Tailwind plugin 🤔 ?

@ahuth ahuth requested review from jinlee93, booc0mtaco and a team March 3, 2023 13:11
@ahuth ahuth changed the base branch from main to next March 3, 2023 13:11
@ahuth ahuth marked this pull request as ready for review March 3, 2023 13:11
@github-actions
Copy link

github-actions bot commented Mar 3, 2023

size-limit report 📦

Path Size
components 121.1 KB (0%)
styles 3.1 KB (0%)

Comment on lines -2 to -3
"trailingComma": "all",
"singleQuote": true,
Copy link
Contributor Author

@ahuth ahuth Mar 3, 2023

Choose a reason for hiding this comment

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

These overrides were the clue that our shared config wasn't being used, since the shared config has the same values.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

So the end result was the same because the overrides, but now we're actually using the correct config?

@ahuth
Copy link
Contributor Author

ahuth commented Mar 3, 2023

So the end result was the same because the overrides, but now we're actually using the correct config?

That's right

@ahuth ahuth merged commit 4ecfc0a into next Mar 3, 2023
@ahuth ahuth deleted the ah-prettier branch March 3, 2023 18:16
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.

2 participants