-
Notifications
You must be signed in to change notification settings - Fork 42
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
Upgrade prettier to 3.2 #296
Conversation
script/screenshots.js
Outdated
@@ -242,7 +245,7 @@ browsers.forEach(function (browser) { | |||
console.log( | |||
'ERROR:', | |||
browser.config.browserName, | |||
browser.config.platform | |||
browser.config.platform, |
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.
I don't know what's going on here, but this feels like prettier is making the wrong decision.
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.
ohh good catch, I think we turned that rule off in our main prettierrc on knox but I didn't do it here.
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.
Looking here it does seem like it's configurable:
https://stackoverflow.com/a/61370655
Someone mentions the rationale behind this is version control working better with trailing commas. I don't have a strong opinion. I think it's best to just match whatever knox does if we've already made a decision there.
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.
Yep.. rebased the first commit to do that and match knox config exactly
c96db0f
to
5b98e19
Compare
Upgrades prettier to version 3.2 (which is what we use on knox as well). Adds the commit which reformats files to git-blame-ignore-revs.