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

Rename trailingComma to trailingCommaPHP #1059

Merged
merged 3 commits into from
May 28, 2019
Merged

Rename trailingComma to trailingCommaPHP #1059

merged 3 commits into from
May 28, 2019

Conversation

flexchar
Copy link
Contributor

Rename trailingComma property to trailingCommaPHP in order to prevent collisions with other Prettier plugins/implementations.

As discussed in #964

@@ -62,7 +62,7 @@ const {
function shouldPrintComma(options, level) {
level = level || "none";

switch (options.trailingComma) {
switch (options.trailingCommaPHP) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's support also options.trailingComma for all and none for better integration

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

All good, one note

@flexchar
Copy link
Contributor Author

Since 'none' is none despite option being present or not, I only add check for 'all'.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

/cc @czosel

Copy link
Collaborator

@czosel czosel left a comment

Choose a reason for hiding this comment

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

👍

@alexander-akait
Copy link
Member

@czosel can you do release? because we are below 1.0 we can release this as minor, don't forget add infromation about this to release page

@czosel
Copy link
Collaborator

czosel commented May 28, 2019

@alexander-akait
Copy link
Member

@czosel yes 👍

@loilo
Copy link
Collaborator

loilo commented May 28, 2019

What about the CLI option? Current v0.11.0 still uses --trailing-comma instead of --trailing-comma-php.

@alexander-akait
Copy link
Member

Need update too

@czosel
Copy link
Collaborator

czosel commented May 28, 2019

Shall I try reverting the release or shall we publish a fix as 0.11.1?

@alexander-akait
Copy link
Member

@czosel yes, looks we do broken release, we should merge this and do release

@czosel
Copy link
Collaborator

czosel commented May 28, 2019

🤦‍♂️ I didn't realize that this wasn't merged yet.

@czosel
Copy link
Collaborator

czosel commented May 28, 2019

I just unpublished 0.11.0 - the current master is good for release, right?

@alexander-akait
Copy link
Member

Yes

@czosel
Copy link
Collaborator

czosel commented May 28, 2019

Published as 0.11.1

@loilo
Copy link
Collaborator

loilo commented May 28, 2019

Alright, playground has been updated. 👍

@flexchar
Copy link
Contributor Author

@evilebottnawi thanks for getting this out!
Did I miss CLI option as mentioned by @loilo? I can't seem to find any code for it thou...

@alexander-akait
Copy link
Member

/cc @loilo

@loilo
Copy link
Collaborator

loilo commented May 28, 2019

I only tested with the 0.11.0 version @czosel published before merging the PR so I guess I actually tested 0.10.2 under the 0.11.0 label. 😁

Since there were no further commits for 0.11.1, I assume that your PR was completely fine @flexchar. ❤️
Sorry for the confusion.

@czosel
Copy link
Collaborator

czosel commented May 29, 2019

I also apologize for the broken release - next time I won’t cut a release while sitting in a meeting 😅

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.

4 participants