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.

Sign up for GitHub

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

Jump to bottom

chore: replace eslint-plugin-prettier with prettier-linter-helpers #372

Merged
merged 2 commits into from Apr 12, 2020
Merged

chore: replace eslint-plugin-prettier with prettier-linter-helpers #372

merged 2 commits into from Apr 12, 2020

Conversation

JounQin
Copy link
Member

@JounQin JounQin commented Apr 9, 2020 •
edited by ikatyang

Fixes #264

@JounQin
Copy link
Member Author

JounQin commented Apr 9, 2020 •
edited

@ikatyang Would you like help to review this PR?

I don't understand why we're testing node v4 on node v6?

ikatyang
ikatyang reviewed Apr 9, 2020
View reviewed changes
Copy link
Member

@ikatyang ikatyang left a comment

Choose a reason for hiding this comment

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

I don't understand why we're testing node v4 on node v6?

The minimal required node version for this package is v4:

tslint-plugin-prettier/package.json

Lines 54 to 56 in 0fc2a4c

"engines": {
"node": ">= 4"
},

And we cannot test node v4 directly:

tslint-plugin-prettier/.travis.yml

Lines 18 to 26 in 0fc2a4c

- > # yarn and tslint-test do not work on node 4
if [ "$TRAVIS_NODE_VERSION" = "6" ]; then
echo '{ "presets": [["@babel/preset-env", { "targets": { "node": "4" } }]] }' > .babelrc.test.json
npm install @babel/cli @babel/core @babel/preset-env
node_modules/.bin/babel node_modules/@babel/code-frame/lib --out-dir node_modules/@babel/code-frame/lib --config-file ./.babelrc.test.json
node_modules/.bin/babel node_modules/@babel/highlight/lib --out-dir node_modules/@babel/highlight/lib --config-file ./.babelrc.test.json
nvm install 4
npm run test
fi

#264 (comment)

This will be a breaking change (require node 6) but it's actually just an internal change so I'll wait for other breaking changes.

package.json Outdated Show resolved Hide resolved
ikatyang
ikatyang reviewed Apr 11, 2020
View reviewed changes
package.json Outdated Show resolved Hide resolved
@ikatyang ikatyang changed the title fix: use prettier-linter-helpers instead of eslint-plugin-prettier chore: replace eslint-plugin-prettier with prettier-linter-helpers Apr 12, 2020
@ikatyang ikatyang changed the base branch from master to next April 12, 2020 04:19
@ikatyang ikatyang merged commit f0887ef into prettier:next Apr 12, 2020
@ikatyang
Copy link
Member

Thanks!

@ikatyang ikatyang mentioned this pull request Apr 12, 2020
Closed
@JounQin
Copy link
Member Author

JounQin commented Apr 21, 2020

@ikatyang Any chance to publish it to npm? On tag next for example?

@ikatyang
Copy link
Member

@JounQin Any reason you want to use it right now? As far as I can tell, there is actually no difference between prettier-linter-helpers and eslint-plugin-prettier.

@JounQin
Copy link
Member Author

JounQin commented Apr 21, 2020 •
edited

@ikatyang I just think it's not reasonable to have two different versions of eslint-plugin-prettier installed which may cause unexpected behavior sometimes if I'm using eslint and tslint at the same time together.

@ikatyang
Copy link
Member

Make sense. I'll see if I have time to publish it tonight.

@ikatyang
Copy link
Member

ikatyang commented Apr 21, 2020 •
edited

👉 https://github.com/prettier/tslint-plugin-prettier/releases/tag/v3.0.0-beta.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ikatyang ikatyang ikatyang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use prettier-linter-helpers
2 participants
@JounQin @ikatyang