-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Prettify #2185
base: master
Are you sure you want to change the base?
Prettify #2185
Conversation
Thanks for the effort! |
Mmm... Table of contents changes were probably due to something gone wrong during merging. Redid everything without including the docs. Line breaks don't affect display though. ### Heading
- List item and ### Heading
- List item Are rendered the same. I think the idea behind these line breaks are improving source readability. Also, AFAIK, for some parser, not surrounding a heading by whitespace is an error (see markdownlint) |
Thanks for the re-run, though I'm still not convinced I like what all it does. For example, removing the close curly brace of the try block from the start of the catch line (see. e.g. examples/create-file.js) seems like poor practice that could open the door to separating those more than they should be separated - as a rule, I always keep those together. The addition of semicolons at the ends of lines generally makes sense, but in some places (like examples/custom-pretty-print.js:10 it seems like this would cause a dirtier diff when altering that pipeline. Removal of line breaks e.g. in examples/delete-level.js and examples/errors.js have a similar impact. The addition of line breaks later in that file seem to harm readability more than it helps, but not in a way that might be easy for a machine following simple rules to detect. The repo settings are both fairly minimal and recent, and I don't have time now to go through each option and get it set right, nor even to go through a detailed review of several hundred changed lines modified by the application of simple rules that sometimes make things worse. |
Prettier barely has settings though. At the end of the day, if you don't like what it does, it should be removed from the repo, otherwise it's just confusing for contributors. It is the modern "Golden standard" though. I would accept all changes for standardization, even if I personally dislike something it does. And if something is really annoying, then you can tweak it and re-run on the repo. You're the boss of course. I don't have real stake at this, after the #2181 is resolved I doubt I'll have anything to contribute any time soon |
I've seen the dialog in #2157 about formatting and decided to see what happens if I run Prettier on the repo. This is the result.
Also added a
.git-blame-ignore-revs
file to exclude myself from git-blame. Unfortunately, there's not way to share git config with the repository, so everyone will need to run the commandgit config blame.ignoreRevsFile .git-blame-ignore-revs
Read more: https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame