-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add prettier #695
Add prettier #695
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #695 +/- ##
==========================================
+ Coverage 92.76% 93.05% +0.28%
==========================================
Files 86 86
Lines 1382 1396 +14
Branches 209 216 +7
==========================================
+ Hits 1282 1299 +17
+ Misses 95 92 -3
Partials 5 5 ☔ View full report in Codecov by Sentry. |
Note: I haven't invoked prettier yet since I'd like to merge the outstanding PRs first (it's gonna create infinite merge conflicts) -- that said we can still review based on the actual changes! I think the most important one is tabs vs spaces. I admit: I prefer tabs, but the actual logic here is prettier defaults to tabs. |
It'd be helpful to add this to a ignore file, per Ignoring mass reformatting commits with git blame! |
535f2ff
to
9a1e414
Compare
9a1e414
to
00fe012
Compare
@jasonaowen ready for real review! |
ESLint is deprecating support for formatting-focused lint rules, suggesting that projects rely on prettiers instead [1]. This commit adds prettier, though the invocation will be separated into another commit to make it easier to resolve merge conflicts that might come up between review and merge. We decided that single quotes was something we collectively wanted to keep, so that is the one setting we're overriding. [1] https://eslint.org/blog/2023/10/deprecating-formatting-rules/
This commit applies prettier to the entire project, which results in massive changes and touches almost every line. Issue #601 Set up prettier
00fe012
to
02c8997
Compare
We recently rewrote almost every line by invoking prettier to format the project. This messes with git blame history, but git provides a handy tool for ignoring that type of commit if the hash is tracked. This commit adds the hash to a tracking file and provides some instructions on how to utilize it.
02c8997
to
4c3fe1c
Compare
}); | ||
}); | ||
it('returns an empty Bundle when no data is present', async () => { | ||
await agent.get('/bulkUploads').set(authHeader).expect(200, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bah -- see this is an example of me not loving the wrapping defaults of prettier (womp womp)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, same. 😞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussion here: prettier/prettier#7884
Note: the failing test is the issue addressed in #707 -- it's not a real failure |
Now that we're using prettier we don't have the lint rules for indentation enabled in the first place, so there is no need to disable them in these strange edge cases.
I added one more commit to remove the now-obsolete eslint disable lines around tab formatting in the context of type helpers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care of this, @slifty!
}); | ||
}); | ||
it('returns an empty Bundle when no data is present', async () => { | ||
await agent.get('/bulkUploads').set(authHeader).expect(200, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, same. 😞
): Promise<Bundle<BulkUpload>> => | ||
loadBundle( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, ew, here's another place where I disagree with Prettier. Should we start wrapping all multiline expressions in parens?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blegh that decision feels so odd to me! I guess I can only hope I'll get used to it...
Oh, and pro tip, you can change the tab width GitHub uses in the appearance settings. |
This PR adds prettier and invokes it on the project.
It also moves us from spaces to tabs (tabs is what prettier has determined to be the Right Choice)
Resolves #601