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

Format markdown files with Prettier #4853

Merged
merged 8 commits into from
Nov 10, 2017
Merged

Conversation

@cpojer
Copy link
Member

cpojer commented Nov 7, 2017

Wow, this is quite extreme but I like it. What do we propose to do in the meantime? Shall we close this PR and wait until prettier is done with all the fixes?

Please exclude the README.md, that one is supposed to be autogenerated from the documentation (not sure if it still is).

@azz
Copy link
Contributor

azz commented Nov 7, 2017

Thanks for raising those issues - we'll get them sorted and publish 1.8.1 asap.

<!--
Before creating an issue please check the following:
* you are using the latest version of Jest
* try re-installing your node_modules folder
* run Jest once with `--no-cache` to see if that fixes the problem you are experiencing.
-->

**Do you want to request a *feature* or report a *bug*?**
**Do you want to request a _feature_ or report a *bug*?**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure why *feature* changed but *bug* didn't? Or is it manual change?

Copy link
Contributor

Choose a reason for hiding this comment

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

@codecov-io
Copy link

codecov-io commented Nov 7, 2017

Codecov Report

Merging #4853 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4853   +/-   ##
=======================================
  Coverage   59.64%   59.64%           
=======================================
  Files         194      194           
  Lines        6502     6502           
  Branches        3        3           
=======================================
  Hits         3878     3878           
  Misses       2624     2624

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 67b4534...d97388c. Read the comment docs.

@azz
Copy link
Contributor

azz commented Nov 7, 2017

Published prettier@1.8.1. Fixes all those issues minus prettier/prettier#3178 (you can use <!-- prettier-ignore --> before the code block to unblock this PR.)

@thymikee
Copy link
Collaborator

thymikee commented Nov 7, 2017

Looks, like there's a PR tackling this last issue (prettier/prettier#3187). I guess we can wait a bit more then.

@SimenB
Copy link
Member Author

SimenB commented Nov 7, 2017

Pushed a new commit which is done from master, but applied on top of 1.8.0 markdown output for easier diff

@cpojer
Copy link
Member

cpojer commented Nov 7, 2017

Let me know when this is ready to merge guys.

@@ -142,5 +142,10 @@
"testMatch": [
"**/*.test.js"
]
},
"prettier": {
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

It was cleaner to put it here instead of inline in the run script

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should be safe to remove all of these except for prettier.eslintIntegration, because eslint --fix is responsible for formatting.

Copy link
Member Author

@SimenB SimenB Nov 8, 2017

Choose a reason for hiding this comment

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

Read the prettier-vscode docs, and the config in there is actually ignored if there's a prettier key in package.json.

https://github.com/prettier/prettier-vscode/blob/221e65c8e245aaea02dc4deb2a566680c3f73caa/README.md#prettiers-settings

(I don't use vscode, which is why I ask :))

@SimenB
Copy link
Member Author

SimenB commented Nov 9, 2017

@cpojer @azz Added commit with prettier 1.8.2 (and rebased on master). Looking pretty good! Nothing jumping out at me

@SimenB
Copy link
Member Author

SimenB commented Nov 10, 2017

@cpojer could you merge before conflicts arise? :D

@cpojer cpojer merged commit 9b40311 into jestjs:master Nov 10, 2017
@cpojer
Copy link
Member

cpojer commented Nov 10, 2017

Done.

@SimenB SimenB deleted the prettier-1.8-md branch November 10, 2017 19:39
@SimenB
Copy link
Member Author

SimenB commented Nov 10, 2017

Woo! Would you be opposed to commit hooks?

@thymikee
Copy link
Collaborator

Just a friendly reminder that commit hooks with Prettier might get terrible while rebasing.

@SimenB
Copy link
Member Author

SimenB commented Nov 10, 2017

True story. Might be enough to periodically prettify

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants