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

Update prettier #22520

Merged
merged 18 commits into from
Apr 15, 2020
Merged

Update prettier #22520

merged 18 commits into from
Apr 15, 2020

Conversation

MichaelDeBoey
Copy link
Contributor

Closes #22506

@MichaelDeBoey MichaelDeBoey requested review from a team as code owners March 23, 2020 23:42
@vladar vladar requested a review from tesseralis April 10, 2020 12:45
Copy link
Contributor

@vladar vladar left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I have one change request:

Prettier 2.0 had changed default value for arrowParens option from "avoid" to "always" (link). But we'd prefer to keep the old style for now.

So could you please add arrowParens: "avoid" to .prettierrc which matches previous default. This should also reduce the number of affected files in this PR.

@MichaelDeBoey MichaelDeBoey force-pushed the update-prettier branch 3 times, most recently from 52464d4 to a31c4d7 Compare April 10, 2020 16:20
@vladar
Copy link
Contributor

vladar commented Apr 14, 2020

Sorry for the delay. I think we can merge this but need to resolve conflicts first and also figure out why the lint step fails. Mind taking a look? Then re-request a review and we'll ship it!

@MichaelDeBoey
Copy link
Contributor Author

@vladar I just rebased my branch onto upstream/master

@vladar
Copy link
Contributor

vladar commented Apr 14, 2020

Yeah but for some reason lint still fails. Does it fail for you locally when running yarn lint:code ? Maybe we need to update eslint-config-prettier too.

@MichaelDeBoey
Copy link
Contributor Author

@vladar I only get warnings when running yarn lint:code when I run it locally.

Will update eslint-config-prettier too, just to be sure 🙂

@MichaelDeBoey
Copy link
Contributor Author

@vladar I've found a linting error, but CI is still failing 😕

@MichaelDeBoey MichaelDeBoey requested a review from a team as a code owner April 14, 2020 21:46
@MichaelDeBoey
Copy link
Contributor Author

@vladar I updated all .prettierrc files and ran Prettier on the full codebase.
I hope this will fix CI

@MichaelDeBoey
Copy link
Contributor Author

@vladar Linting is passing, but still errors on unit tests.
Can't figure out what causes them to fail tho 😕

@vladar
Copy link
Contributor

vladar commented Apr 15, 2020

I think tests are failing because prettier output is captured in snapshots in several plugins. You need to run yarn run jest --updateSnapshot in the repo root to update them all.

Also, sadly, there are new conflicts. Looks like we must move fast after changes to be able to merge this 😄

@@ -173,7 +173,8 @@ To write a new GraphQL example, a Codesandbox project with a Gatsby site can be
```mdx
Here's an example of a GraphQL query inline:

<iframe src="https://711808k40x.sse.codesandbox.io/___graphql?query=query%20TitleQuery%20%7B%0A%20%20site%20%7B%0A%20%20%20%20siteMetadata%20%7B%0A%20%20%20%20%20%20title%0A%20%20%20%20%7D%0A%20%20%7D%0A%7D%0A&explorerIsOpen=false&operationName=TitleQuery" /> // highlight-line
<iframe src="https://711808k40x.sse.codesandbox.io/___graphql?query=query%20TitleQuery%20%7B%0A%20%20site%20%7B%0A%20%20%20%20siteMetadata%20%7B%0A%20%20%20%20%20%20title%0A%20%20%20%20%7D%0A%20%20%7D%0A%7D%0A&explorerIsOpen=false&operationName=TitleQuery" /> //
highlight-line
Copy link
Contributor

Choose a reason for hiding this comment

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

Moving highlight-line to the new line doesn't look right.

Copy link
Contributor

Choose a reason for hiding this comment

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

this got merged with the new line

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for noticing! Created a PR to address this: #23140

@MichaelDeBoey
Copy link
Contributor Author

@vladar I just rebased the branch and tried to run yarn test:update in the root, but that throws a lot of errors:

 Cannot find module 'gatsby-core-utils' from 'path-serializer.ts'

Can't figure out what causes this error tho 😕

@vladar
Copy link
Contributor

vladar commented Apr 15, 2020

Maybe try running yarn bootstrap first?

package.json Outdated Show resolved Hide resolved
MichaelDeBoey and others added 2 commits April 15, 2020 17:21
Co-Authored-By: Dima An <dmitriym44@gmail.com>
Copy link
Contributor

@vladar vladar left a comment

Choose a reason for hiding this comment

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

Finally, it's green! Sorry for the churn but it's done now! Let's ship it while we can :) Great job! 💜

@vladar vladar added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Apr 15, 2020
@gatsbybot gatsbybot merged commit ff911e2 into gatsbyjs:master Apr 15, 2020
@MichaelDeBoey
Copy link
Contributor Author

I'm happy it got green 🙂

@MichaelDeBoey MichaelDeBoey deleted the update-prettier branch April 15, 2020 16:03
pragmaticpat pushed a commit to pragmaticpat/gatsby that referenced this pull request Apr 28, 2022
* Update prettier to latest version

* Run 'yarn format' from root of project

* Remove prettier-ignore for nested markdown

* Fix starters

* Fix benchmarks

* Fix examples

* Fix integration-tests

* Update eslint-config-prettier & eslint-plugin-prettier

* Fix linting

* Update all .prettierrc

* Run Prettier on full codebase

* Remove unnecessary .prettierrc & .prettierignore files

* Update Prettier script

* Update snapshots

* Revert snapshot

* Revert "Update Prettier script"

This reverts commit 008eb99

* Update snapshot

* Update docs/blog/2018-08-09-swag-store/index.md

Co-Authored-By: Dima An <dmitriym44@gmail.com>

Co-authored-by: Dima An <dmitriym44@gmail.com>
mwfrost pushed a commit to mwfrost/gatsby that referenced this pull request Apr 20, 2023
* Update prettier to latest version

* Run 'yarn format' from root of project

* Remove prettier-ignore for nested markdown

* Fix starters

* Fix benchmarks

* Fix examples

* Fix integration-tests

* Update eslint-config-prettier & eslint-plugin-prettier

* Fix linting

* Update all .prettierrc

* Run Prettier on full codebase

* Remove unnecessary .prettierrc & .prettierignore files

* Update Prettier script

* Update snapshots

* Revert snapshot

* Revert "Update Prettier script"

This reverts commit 008eb99

* Update snapshot

* Update docs/blog/2018-08-09-swag-store/index.md

Co-Authored-By: Dima An <dmitriym44@gmail.com>

Co-authored-by: Dima An <dmitriym44@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(docs) Upgrade to Prettier 2.0
5 participants