Skip to content

Conversation

@czosel
Copy link
Collaborator

@czosel czosel commented Apr 9, 2018

Fixes #356

@czosel czosel requested a review from mgrip April 9, 2018 19:29
README.md Outdated
yarn:

```bash
yarn add --dev prettier/prettier prettier/plugin-php
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or should we prefer global installation?

Copy link
Member

Choose a reason for hiding this comment

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

@czosel Let's do this as do prettier here https://prettier.io/docs/en/install.html

@czosel czosel requested a review from alexander-akait April 9, 2018 19:31
@czosel czosel force-pushed the readme-refactor branch 2 times, most recently from 3c1ad79 to e731144 Compare April 9, 2018 19:40
README.md Outdated
and then run it via

```bash
yarn run prettier path/to/file.php
Copy link
Contributor

Choose a reason for hiding this comment

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

still need the --write here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍 fixed

@czosel czosel force-pushed the readme-refactor branch from e731144 to 89d6f53 Compare April 9, 2018 19:42
README.md Outdated

```json
scripts: {
"prettier":"prettier"
Copy link
Member

Choose a reason for hiding this comment

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

Do we need glob ({**/*,*}.php) here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not when you're calling it like instructed below: yarn run prettier path/to/file.php --write

Copy link
Contributor

@mgrip mgrip left a comment

Choose a reason for hiding this comment

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

looks great! should we leave this open until we verify all of this actually works? 😆

@czosel
Copy link
Collaborator Author

czosel commented Apr 9, 2018

I just tried the local installation with prettier/prettier and prettier/plugin-php. After the release, it should work with @prettier/plugin-php. I'd say let's replace prettier with prettier/prettier for now, and do the release 😉

@mgrip
Copy link
Contributor

mgrip commented Apr 9, 2018

works for me!

cc @azz @vjeux just a heads up - this means we're telling people to add prettier as a dependency from github rather than npm. Not sure if we should be concerned about that, other option would mean waiting for next prettier release

@czosel
Copy link
Collaborator Author

czosel commented Apr 9, 2018

Alright, let's do this 😄

Most important question: What are we calling the version? 0.1? 0.1-alpha?

@mgrip
Copy link
Contributor

mgrip commented Apr 9, 2018

My vote would be 0.1.0 I think? https://semver.org/#how-should-i-deal-with-revisions-in-the-0yz-initial-development-phase

@mgrip
Copy link
Contributor

mgrip commented Apr 9, 2018

@czosel I'd say feel free to publish on npm and update the package.json as part of this PR

@czosel
Copy link
Collaborator Author

czosel commented Apr 9, 2018

Done 🎉
https://www.npmjs.com/package/@prettier/plugin-php

@mgrip
Copy link
Contributor

mgrip commented Apr 9, 2018

Amazing 🎉 awesome work guys!

@czosel czosel merged commit 6075752 into prettier:master Apr 9, 2018
@czosel czosel deleted the readme-refactor branch April 9, 2018 20:13
@azz
Copy link
Member

azz commented Apr 10, 2018

Awesome! 🎉

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