-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Put code style settings for IntelliJ IDEA under version control #426
Comments
I've submitted some pull requests and I've noticed that the code style is very inconsistent. For example the use of curly brackets, indenting and some other things. I see code style as part of code qualtiy. Is it possible that the JUnit team commits the code style settings to keep the quality high? |
The formatting settings are already committed for Eclipse users: https://github.com/KentBeck/junit/blob/master/.settings/org.eclipse.jdt.core.prefs Do you think we should require submitters to import the project to Eclipse for formatting? I could certainly go along with that. |
I've overlooked the Eclipse file :) I use Intellij IDEA. Is it an option to commit the a formatting file for Intellij IDEA as well? |
I think it could be really useful if someone took a look at the style of especially most of the junit.framework classes, and found formatter settings for IDEA that would maintain them. I'd be happy to accept such a patch. |
I will see what I can do |
Is it possible to change the code style a bit like the missing space before the equal sign? |
Arjan, Yes, I'd be open to that (the current style is, I believe, an amalgam between IBM Java style and SmallTalk.) But I'd want to update all the code to the new style at once, which would be quite an undertaking. |
I understand that you want to update all the code at once but I think that with the IDE nowadays that it isn't a problem. |
I'm not sure what you mean by "isn't a problem". If we adjust the formatting rules without also bringing the code into line, we'll end up with a situation where > 50% of the lines in each change are just adjusting whitespace, which is hard to review. |
What I actually mean is that it's easy with the modern IDE's the apply a (new) coding style to a whole project. So reformatting the current code base is not difficult. |
Got it. Is it easy enough to try the Sun coding conventions, issue a pull David Saff On Tue, Aug 21, 2012 at 4:27 PM, Arjan [email protected] wrote:
|
I reformatted the code (indenting, use of curly braces, use of whitespace) and then I ran Checkstyle with the sun_checks.xml configuration file. There are 3342 violations. A lot of line length and javadoc style violations. There are also al lot of warnings about parameters that should be final. Before submitting a pull request I think it must be clear how to deal with these violations. One approach is to modify the sun_checks.xml file to disable the checks that are irrelevant to the JUnit team or another approach can be to deal with all these warnings. I would actually have choosen the first approach. Or just leave out the checkstyle check :) |
Reformatted the code base for an consistent coding/formatting style
Issue junit-team#426 Description of the coding style
Issue junit-team#426 Removed confusing whitespaces.
Removed the Markdown syntax, changed import order and removed confusing Maximum Blank Lines section.
- Do not align JavaDoc parameter description - Removed "Keep empty .. JavaDoc tag" - corrected typp
issue #426 Description of the coding style
Applied the new coding style to the code base for issue junit-team#426.
Fixed by #522. |
Arjan, Do you currently have an Eclipse formatter settings file that we can check in to help others who want to follow this style? Thanks. |
Oops, sorry, I thought those were committed in #522. Reopening the issue... |
@awulder Ping! |
@marcphilipp I've been very busy lately. I don't use Eclipse but I will commit the formatter setting for Eclipse and Intellij IDEA. |
@awulder Sounds good! If you don't find enough time, please let us know. It would be great to finally make formatting a non-issue in pull requests. |
@awulder Have you started to work on this or should someone else take over? |
Does anybody have a spare time to check the *.prefs files in |
It would be convenient to enforce code style conventions (like imports) by committing them as Eclipse 'project specific settings' into the VCS. The Eclipse project settings are already under version control anyway. That way, new and unwary developers that wish to contribute to the project automatically use the correct code style.
The text was updated successfully, but these errors were encountered: