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

Clarify Prettier defaulting behavior #616

Merged
merged 2 commits into from Oct 7, 2021
Merged

Conversation

devinrhode2
Copy link
Contributor

@devinrhode2 devinrhode2 commented Oct 6, 2021

This definitely sent me down a rabbit hole: #614

TODO:

  1. Cleanup source for bracketSameLine. XO Does not need to set it to false, since that is prettier's current default. (I'm assuming it was different in the past).
    2. Verify this actually works, check when errors are actually thrown
    Changing bracketSpacing actually works! :)

This definitely sent me down a rabbit hole: xojs#614
@devinrhode2
Copy link
Contributor Author

devinrhode2 commented Oct 6, 2021

  1. could be moved to it's own issue, and this PR could be merged now

@sindresorhus sindresorhus marked this pull request as ready for review Oct 7, 2021
@sindresorhus sindresorhus changed the title Clarify prettier defaulting behavior Clarify Prettier defaulting behavior Oct 7, 2021
@sindresorhus sindresorhus merged commit eefd88a into xojs:main Oct 7, 2021
3 checks passed
@sindresorhus
Copy link
Member

sindresorhus commented Oct 7, 2021

Cleanup source for bracketSameLine. XO Does not need to set it to false, since that is prettier's current default. (I'm assuming it was different in the past).

PR welcome

@devinrhode2 devinrhode2 deleted the patch-1 branch Oct 7, 2021
@devinrhode2
Copy link
Contributor Author

devinrhode2 commented Oct 7, 2021

I would argue that XO should not be changing these defaults, but it's probably not worth the breaking change for existing consumers. Maybe there's a way to detect an upgrade and then just modify the users prettier config, or just print a warning.

@sindresorhus
Copy link
Member

sindresorhus commented Oct 8, 2021

Changing the defaults is very much intentional.

devinrhode2 added a commit to devinrhode2/xo that referenced this issue Oct 15, 2021
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
devinrhode2 added a commit to devinrhode2/xo that referenced this issue Oct 15, 2021
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
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.

None yet

2 participants