-
Notifications
You must be signed in to change notification settings - Fork 19
Add EditorConfig file #180
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
Conversation
|
While I agree we should add something like this. I think we are considering palantir-java-formatter (see #183). |
|
Editorconfig is on a lower level. I am not sure that any formatter plugin with necessarily handle everything so you may want to use both as they do different things. Thoughts @ppkarwasz? Editorconfig can be used to check some of the formatting also. I am a contributor to both https://github.com/ec4j/editorconfig-maven-plugin. But, it is more meant to set the IDE settings for the developers and is not language-specific as it will make sure that the correct indentation level is set for various languages like JSON, Shell, and YAML, too, not just Java. I don't have a strong opinion on which formatter plugin to choose. I have contributed to https://github.com/revelc/formatter-maven-plugin and I think that https://github.com/diffplug/spotless is one of the more popular ones. |
|
The goods news is that
|
OK, palantir-java-format uses spotless. |
ppkarwasz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add a .gitattributes file, so Windows users don't end up checking out CRLFs?
I usually use something like this:
# All text files with LF line endings
* text=auto eol=lf
# Maven Wrapper cmd script
/mvnw.cmd eol=crlf
# Maven Wrapper need LF line endings
/.mvn/wrapper/maven-wrapper.properties eol=lf
I don't know how Git detects which files are text, but this repo should not contain binary blobs, so it should be fine.
.editorconfig
Outdated
| [{*.json,*.sh,*.yml}] | ||
| indent_size = 2 | ||
| tab_width = 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add *.xml and *.yaml too? I find that XML looks better with 2 spaces of indentation. This is also what sortpom uses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I can add .yaml, but so far, the files all use .yml. I am not sure if the 3-letter or 4-letter extension is preferred over the other.
About .xml, pom.xml was currently set to 4 spaces. I like the sortpom plugin, too, and it looks like 2 is the canonical number anyway.
It's easier, I guess, to just change the default to 2, and set 4 for .java, then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, 6bf3a95 just defaults to 2.
In some repo I had to disable |
Does spotless support all of the same features? If so, it could fix any issues without an additional plugin. |
I thought that this has more to do with the I think that ideally all files should be treated as binary and it shouldn't be converting anything and files should be kept as-is. Windows files should stay |
OK, see #190. |
I am not sure if all are supported, but there are some generic formatting steps and we can use the Prettier formatter for the rest. |
When Windows users create a new Java file, they might use CRLF. If we add a |
I prefer to treat all text as binary (that's what I did in #190). This means git won't mess with text files. Otherwise, you have to specify every Windows file in the The formatter plugin could be used to check the line ending on a pull request, so does git even need to get involved? No one should be committing directly, except maybe a upon a new release. |
No description provided.