Add Makefile and .json to editorconfig#30324
Conversation
merwok
left a comment
merwok
left a comment
There was a problem hiding this comment.
I would decline this PR, I don’t see that it solves a problem.
| [Makefile{,.pre.in,.pre}] | ||
| indent_size = 4 | ||
| indent_style = tab | ||
| trim_trailing_whitespace = true |
There was a problem hiding this comment.
I would question an editor that does not properly handle makefiles!
There was a problem hiding this comment.
That's something we cannot control 🙂
There was a problem hiding this comment.
Sure, this is harmless to have, why not add it.
|
|
||
| [*.{py,c,cpp,h}] | ||
| [*.{py,c,cpp,h,json}] | ||
| indent_size = 4 |
There was a problem hiding this comment.
Formatting JSON files does not seem necessary for this repo. I have only found a few, and they seem to be imported from external sources or maintained without issue.
There was a problem hiding this comment.
We have 11 json files in the repo: https://github.com/python/cpython/search?q=language%3Ajson
I don't think json support is any different from yml, which we already have.
There was a problem hiding this comment.
I know, that’s what I called a few, and you’re ignoring my second note that they are mostly copied from external sources.
I am -1 on this change. It does not solve any problem.
There was a problem hiding this comment.
Sorry, not meant to ignore your words at all! 🙂
It was more like a personal research, because I know that are not-so-many json files, but I was not sure about the exact amount.
I also agree that this is not a real "problem" at all, no one was ever hurt by the wrong number of spaces in a json file (at least I really hope so!).
But, the initial reason I've created this PR was this file in its current form: https://github.com/python/cpython/blob/main/.github/problem-matchers/msvc.json
I was reading through it and asked myself: why does it have mixed formatting? What is the rule? I looked at .editorconfig, but didn't find any. And since I was also fixing .editorconfig files in other python/* repositories, I've decided to create a PR that can potentially solve this tiny problem in the future.
But, I don't have strong feelings about a so-little-change, so I / you / anyone-else can totally close this PR if needed 👍
Thanks a lot for your feedback and review, TIL about tabstop!
There was a problem hiding this comment.
With an old, stable codebase like CPython, the team prefers to solve real problems (reported by end users or core developers) and avoid cosmetic changes that don’t provide a concrete improvement, may introduce errors and muddle history (git log is not very good, not to mention github history that doesn’t even follow renames). Consistency in itself and for itself is not a goal! Thanks for your efforts, they will be very appreciated for other kinds of PRs!
Co-authored-by: Éric <merwok@netwok.org>
|
The changes for JSON are not wanted, and the remaining changes for makefiles seem too small to warrant a commit. |
5 participants
There are quite a lot of
jsonfiles that were missing from this definition.I've also added all
Makefiles to the spec, because it is important to usetabs when editing them.Docs: https://editorconfig.org/
Refs #27638
Refs https://bugs.python.org/issue44854
CC @ambv