build: add prettier to format JSON, MD and YAML files #41880
Conversation
alan-agius4
added
area: build & ci
|
This change will require a larger conversation as we have a few different teams as stakeholders, notably the docs team whose main medium is the |
| third_party/ | ||
| aio/content/examples/ | ||
| aio/tools/transforms/templates/ | ||
| /CHANGELOG.md |
There was a problem hiding this comment.
We should ignore all CHANGELOG.md files as we have one in zone.js as well
Currently there are 1419 JSON, MD and YAML files which are not formatted, with this change we add prettier to format them when a file is changed.
| @@ -0,0 +1,6 @@ | |||
| { | |||
| "printWidth": 100, | |||
There was a problem hiding this comment.
FWIW, currently we refrain from wrapping Markdown files at 100 chars (as it makes diffs quite verbose).
We try to keep one sentence per line, but I am not sure if prettier supports that 🤔
There was a problem hiding this comment.
I agree with @gkalpak here (as I think will @IgorMinar) - but LOL @jelbourn was just saying in a stand-up this week how he dislikes long lines in markdown. (Clearly does not have wrapping turned on in his IDE).
There was a problem hiding this comment.
The printWidth setting doesn't apply to MarkDown files.
There was a problem hiding this comment.
The
printWidthsetting doesn't apply to MarkDown files.
Are you sure? The prettier playground seems to indicate otherwise 😕
There was a problem hiding this comment.
Umm.. for some reason locally it didn’t have any effect when I tried it earlier. Will take another look later on.
That said, if this is applied to markdowns I believe there is no way to have different print widths for different file types. (Example: 100 for TS and JS and unlimited for MD).
There was a problem hiding this comment.
I do feel pretty strongly about line breaking. I find having overflowing lines more difficult to read, edit, and review than breaking. Our internal markdown docs tool also enforces breaking at 100 columns and I've never found that difficult to review.
There was a problem hiding this comment.
@jelbourn Have you tried soft wrap? https://www.jetbrains.com/webstorm/guide/tips/soft-wraps/
GitHub automatically soft wraps as well. So if you configure your editor the file contents look great.
The biggest benefit of sentence per line formating is that if you add or remove a word in a sentence you don't trigger reformatting of the entire paragraph, instead all code review tools will highlight only the modified sentence with the added/removed word with extra emphasis.
There was a problem hiding this comment.
Yeah, I just have a preference for not using soft wraps (since I don't like there being a disconnect between what I see and actual text-in-file).
Maybe I'm weird, but when a paragraph changes, I feel like you would generally re-read the surrounding text as well anyway to make sure the whole thing still makes sense. I don't see a reformatting as meaningfully changing the review burden in any way.
There was a problem hiding this comment.
I think it would be the best if the folks that do edit and review these docs the most - @aikidave and the tech writing crew - made this decision.
I think there is a lot of value in keeping the diffs minimal and clean, but Dave is the ultimate stakeholder here so we should adjust to his preferences.
There was a problem hiding this comment.
I am going to wait on @aikidave response before doing any changes.
| "printWidth": 100, | ||
| "quoteProps": "preserve", | ||
| "singleQuote": true, | ||
| "trailingComma": "all" |
There was a problem hiding this comment.
another one to consider is "bracketSpacing": false: https://prettier.io/docs/en/options.html#bracket-spacing
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Currently there are 1419 JSON, MD and YAML files which are not formatted, with this change we add prettier to format them when a file is changed.