Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

Prettify docs files #2275

Closed
wants to merge 4 commits into from
Closed

Prettify docs files #2275

wants to merge 4 commits into from

Conversation

yangshun
Copy link
Contributor

@yangshun yangshun commented Dec 4, 2019

Summary

Prettier can also be used to format more than just JS files, it can format Markdown files too. Instead of using Flow parser for all files in prettier.config.js, we can just limit Flow parser for JS files and let Prettier find the default parsers for the various file formats.

Hence I tweaked the Prettier config and added overrides for:

  • JS format - Use Flow parser
  • Markdown format - Restrict to 80 characters so that it's easier to read

I then ran Prettier on all the docs. I've added a prettier:diff command into the lint-docs step, ran alongside with Alex that will fail when there are unformatted docs. It's not part of the CI step as I think that might be a bit too strict given that some people (including myself) edit docs in the GitHub UI and that might cause formatting issues which can be fixed by running the format-docs command every now and then.

The formatting is only restricted to the docs now and not code as it's potentially dangerous to format code that gets synced with internal FB repos.

Test Plan

Load website.

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@yangshun has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@mrkev
Copy link
Contributor

mrkev commented Dec 4, 2019

Oh this is cool. Ideally yeah, let's enforce it in lint-docs. Can prettier automatically apply formatting to markdown files too? Adding that in format or a new script would be nice.

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@yangshun has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@@ -26,8 +26,10 @@
"dev": "gulp dev",
"postbuild": "node node_modules/fbjs-scripts/node/check-lib-requires.js lib",
"lint": "eslint .",
"lint-docs": "alex .",
"lint-docs": "alex . && yarn format-docs:diff",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if prettier differences are considered part of linting?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep that's legit, the && enforces a clean Alex pass before Prettier

{
files: '*.js',
options: {
parser: 'flow',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use Flow only for JS so that other filetypes can be formatted by Prettier using their respective parsers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are there other files that should be formatted by flow? e.g. .json or inlined into .html files?

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@yangshun has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

If you have changed any documentation, before submitting your PR, please run the following command from the root of the directory to use Prettier to format the documentation.

```
$ yarn format-docs
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@yangshun has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link

@yangshun merged this pull request in 6fc9964.

mmissey pushed a commit to mmissey/draft-js that referenced this pull request Mar 24, 2020
Summary:
**Summary**

Prettier can also be used to format more than just JS files, it can format Markdown files too. Instead of using Flow parser for all files in `prettier.config.js`, we can just limit Flow parser for JS files and let Prettier find the default parsers for the various file formats.

Hence I tweaked the Prettier config and added overrides for:

- JS format - Use Flow parser
- Markdown format - Restrict to 80 characters so that it's easier to read

I then ran Prettier on all the docs. I've added a `prettier:diff` command into the `lint-docs` step, ran alongside with Alex that will fail when there are unformatted docs. It's not part of the CI step as I think that might be a bit too strict given that some people (including myself) edit docs in the GitHub UI and that might cause formatting issues which can be fixed by running the `format-docs` command every now and then.

The formatting is only restricted to the docs now and not code as it's potentially dangerous to format code that gets synced with internal FB repos.

**Test Plan**

Load website.
Pull Request resolved: facebookarchive#2275

Reviewed By: mrkev

Differential Revision: D18807420

Pulled By: yangshun

fbshipit-source-id: b3f34282c2a2cad8e89cd90e8f3f6b3860b747fc
vilemj-Viclick pushed a commit to kontent-ai/draft-js that referenced this pull request Jul 16, 2020
Summary:
**Summary**

Prettier can also be used to format more than just JS files, it can format Markdown files too. Instead of using Flow parser for all files in `prettier.config.js`, we can just limit Flow parser for JS files and let Prettier find the default parsers for the various file formats.

Hence I tweaked the Prettier config and added overrides for:

- JS format - Use Flow parser
- Markdown format - Restrict to 80 characters so that it's easier to read

I then ran Prettier on all the docs. I've added a `prettier:diff` command into the `lint-docs` step, ran alongside with Alex that will fail when there are unformatted docs. It's not part of the CI step as I think that might be a bit too strict given that some people (including myself) edit docs in the GitHub UI and that might cause formatting issues which can be fixed by running the `format-docs` command every now and then.

The formatting is only restricted to the docs now and not code as it's potentially dangerous to format code that gets synced with internal FB repos.

**Test Plan**

Load website.
Pull Request resolved: facebookarchive#2275

Reviewed By: mrkev

Differential Revision: D18807420

Pulled By: yangshun

fbshipit-source-id: b3f34282c2a2cad8e89cd90e8f3f6b3860b747fc
alicayan008 pushed a commit to alicayan008/draft-js that referenced this pull request Jul 4, 2023
Summary:
**Summary**

Prettier can also be used to format more than just JS files, it can format Markdown files too. Instead of using Flow parser for all files in `prettier.config.js`, we can just limit Flow parser for JS files and let Prettier find the default parsers for the various file formats.

Hence I tweaked the Prettier config and added overrides for:

- JS format - Use Flow parser
- Markdown format - Restrict to 80 characters so that it's easier to read

I then ran Prettier on all the docs. I've added a `prettier:diff` command into the `lint-docs` step, ran alongside with Alex that will fail when there are unformatted docs. It's not part of the CI step as I think that might be a bit too strict given that some people (including myself) edit docs in the GitHub UI and that might cause formatting issues which can be fixed by running the `format-docs` command every now and then.

The formatting is only restricted to the docs now and not code as it's potentially dangerous to format code that gets synced with internal FB repos.

**Test Plan**

Load website.
Pull Request resolved: facebookarchive/draft-js#2275

Reviewed By: mrkev

Differential Revision: D18807420

Pulled By: yangshun

fbshipit-source-id: b3f34282c2a2cad8e89cd90e8f3f6b3860b747fc
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.

4 participants