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

Update dependency prettier to v3 #8241

Closed
wants to merge 1 commit into from
Closed

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Jul 8, 2023

Mend Renovate

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
prettier (source) 2.8.8 -> 3.0.0 age adoption passing confidence

Release Notes

prettier/prettier (prettier)

v3.0.0

Compare Source

diff

🔗 Release Notes


Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.


  • If you want to rebase/retry this PR, check this box

This PR has been generated by Mend Renovate. View repository job log here.

@renovate renovate bot added dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code, used by dependency tooling skip-changelog Should not be shown in the changelog labels Jul 8, 2023
@renovate renovate bot requested a review from a team July 8, 2023 13:09
@daniel-beck
Copy link
Member

daniel-beck commented Jul 9, 2023

Interesting, all the changes seem to be for the default value for trailingComma changed to all.

It appears this only exists for nicer diffs; which we've basically abandoned for this repo with the Spotless line length/breaking rules. Would it be preferable to change this option?

@NotMyFault
Copy link
Member

Interesting, all the changes seem to be for the default value for trailingComma changed to all.

It appears this only exists for nicer diffs; which we've basically abandoned for this repo with the Spotless line length/breaking rules. Would it be preferable to change this option?

The new default didn't work out with HTMLUnit before. Hence, the preexisting trailing commas were used sporadically and not covered by rules.
I removed the trailing commas in place and added a rule, HTMLUnit likes that change.

@timja
Copy link
Member

timja commented Jul 10, 2023

The new default didn't work out with HTMLUnit before

Where? Seems a bit of an odd one it's very standard to leave trailing comma's in JavaScript. and yes it does give nicer diffs

@NotMyFault
Copy link
Member

The new default didn't work out with HTMLUnit before

Where? Seems a bit of an odd one it's very standard to leave trailing comma's in JavaScript. and yes it does give nicer diffs

=> #8244

@timja
Copy link
Member

timja commented Jul 10, 2023

The new default didn't work out with HTMLUnit before

Where? Seems a bit of an odd one it's very standard to leave trailing comma's in JavaScript. and yes it does give nicer diffs

=> #8244

Was that automated? it looks like https://github.com/jenkinsci/jenkins/pull/8244/files#r1258086417 is just invalid.

@NotMyFault
Copy link
Member

NotMyFault commented Jul 10, 2023

The new default didn't work out with HTMLUnit before

Where? Seems a bit of an odd one it's very standard to leave trailing comma's in JavaScript. and yes it does give nicer diffs

=> #8244

Was that automated? it looks like #8244 (files) is just invalid.

Yeah, IJ ran yarn run lint:fix. I didn't look over the diff in detail.

@timja
Copy link
Member

timja commented Jul 10, 2023

could probably be fixed by slightly adjusting that one line of code I expect.

Trailing comma's are much nicer for diffs.

@daniel-beck
Copy link
Member

daniel-beck commented Jul 10, 2023

Trailing comma's are much nicer for diffs.

We've abandoned the idea of nice diffs in favor of Spotless's rigid line break rules. Why does it matter here?

@timja
Copy link
Member

timja commented Jul 10, 2023

We've abandoned the idea of nice diffs in favor of Spotless's rigid line break rules. Why does it matter here?

in the inverse why not have it look better when we can for no cost? There doesn't seem to be a technical reason to change a default value here.

@daniel-beck
Copy link
Member

daniel-beck commented Jul 10, 2023

no cost

It's a pretty awkward looking and easy to misread syntax, but TBF that may be my unfamiliarity with it. So minor downside (IMO) to no upside in this repo 🤷

Anyway, nonblocking, just seems wildly inconsistent.

.prettierrc.json Outdated
@@ -1,4 +1,5 @@
{
"endOfLine": "auto",
"htmlWhitespaceSensitivity": "ignore"
"htmlWhitespaceSensitivity": "ignore",
"trailingComma": "none"
Copy link
Member

Choose a reason for hiding this comment

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

My vote is to stay with the Spotless default rather than introduce our own customizations which then need to be maintained.

@github-actions github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Jul 12, 2023
@github-actions
Copy link
Contributor

Please take a moment and address the merge conflicts of your pull request. Thanks!

@github-actions github-actions bot removed the unresolved-merge-conflict There is a merge conflict with the target branch. label Jul 13, 2023
@renovate renovate bot deleted the renovate/prettier-3.x branch July 27, 2023 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code, used by dependency tooling skip-changelog Should not be shown in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants