Community
Participate
Working Groups
With the implementation of bug 339915 to create an eclipse-parent pom file. We now have the ability to fairly easily have PMD or Checkstyle maven plugins to enable code quality checks during a build. One of the things that can be difficult at times when contributing back to an open source project is making sure the source code is consitently formatted the same ways. We have many style checks that happen with JDT, but it would be nice to have an agreed upon standard across the projects at eclipse. Projects should have the ability to override these when necessary, but in general it would help to have an established baseline that can be run and code quality reports run. By default Checkstyle uses the Sun recommendations, however, I know the Eclipse platform has extended these. If other projects have additional requirements we should gather them together, come to an agreed upon baseline, and then setup PMD/Checkstyle to report any violations that occur. You can then have Hudson fail the build, or make it unstable if more than a certain threshold level of warnings occur.
I want to avoid *imposing* any particular style on projects. I am in favour of recommendations or best practices (I am getting disturbingly comfortable with this term).
(In reply to comment #1) > I want to avoid *imposing* any particular style on projects. I am in favour of > recommendations or best practices (I am getting disturbingly comfortable with > this term). Okay, we can call them "recommendations", but still should have some recommendations that can be enabled via Profiles during a build. Having something consistent to follow just makes life easier in the long run.
(In reply to comment #0) > One of the things that can be difficult at times when contributing back to an > open source project is making sure the source code is consitently formatted the > same ways. We have many style checks that happen with JDT, but it would be > nice to have an agreed upon standard across the projects at eclipse. Can you elaborate on what's difficult about different projects having different coding conventions? Here is what most Eclipse Platform and Equinox teams do: - Each team settles on formatting conventions they can live with - The code formatter profile is configured in the *project* settings so they are checked into source control along with the source - The "Format on save" Editor hook is used to make sure that all source code is consistently formatted. This way I can contribute to several projects and not worry about making sure I format the code according to the conventions of each particular project. In my experience you'll never find more than about 5 developers that can agree on a set of formatting conventions. I would be -1 on the arch council even attempting to come up with specific formatting conventions that we recommend to projects - there's just no way we will all agree, and there really is no significant benefit to enforcing conventions across different teams. For an example of the massive time sink this would be, see bug 258729, where a single team took a year to agree on a new value for a single formatter option (line width setting).
Let me clarify a little bit. Eclipse has a standard set of default formatting items that can be applied. What I was thinking is that this can be the base line. Then, if projects want to deviate from the base line, they can do so, and can provide a profile during their build to make sure that all their project committers are following the same standard. So if we can agree at least on the eclipse baseline being what the eclipse JDT sets as defaults, then we have a starting point. I do agree that the formatting guidelines should be control and set by the project, but some general "best practices" would seem to be good. As for why a project should do this, it just makes accepting patches and applying them much easier. I don't know how many times I've had to deal with patches that won't apply cleanly because a users formatting is different than what the project source codes formatting is. And yes, all committers on a project should come to a consensus on what the formatting guidelines are, and stick with them. Even if they don't necessarily agree with them. Again, projects should set what they want, but we should provide some guidance on this. Surely the Architecture Council can come to some basic agreements and recommendations for projects.
(In reply to comment #3) > Can you elaborate on what's difficult about different projects having different > coding conventions? Here is what most Eclipse Platform and Equinox teams do: > > - Each team settles on formatting conventions they can live with > - The code formatter profile is configured in the *project* settings so they > are checked into source control along with the source > - The "Format on save" Editor hook is used to make sure that all source code is > consistently formatted. > > This way I can contribute to several projects and not worry about making sure I > format the code according to the conventions of each particular project. Right, but most projects outside of the Eclipse Platform don't enable this, and it is always a good idea to have an option in a build to check this stuff. In addition, check style and pmd can check to make sure that the appropriate headers are added to files.
(In reply to comment #0) > > By default Checkstyle uses the Sun recommendations, however, I know the Eclipse > platform has extended these. If other projects have additional requirements > we should gather them together, come to an agreed upon baseline, and then setup > PMD/Checkstyle to report any violations that occur. We are using Checkstyle on our project and in our company. We've had a hell of a fight to get an agreement on formatting with 30 developers involved. Sorry, I don't see any kind of agreement possible with a thousand people arguing about style. > > You can then have Hudson fail the build, or make it unstable if more than a > certain threshold level of warnings occur. And I would never impose Checkstyle on an existing code base. Even in well behaved code one would get thousands (more likely tens of thousand) warnings/errors. This said I'm in favour of trying to do this, even if it's only a "best practice".
(In reply to comment #6) > (In reply to comment #0) > > > > By default Checkstyle uses the Sun recommendations, however, I know the Eclipse > > platform has extended these. If other projects have additional requirements > > we should gather them together, come to an agreed upon baseline, and then setup > > PMD/Checkstyle to report any violations that occur. > > We are using Checkstyle on our project and in our company. We've had a hell of > a fight to get an agreement on formatting with 30 developers involved. Sorry, I > don't see any kind of agreement possible with a thousand people arguing about > style. I don't expect everybody to agree to what should be the best practice, but I do think that there can be a common ground on what should be included as the base line. > And I would never impose Checkstyle on an existing code base. Even in well > behaved code one would get thousands (more likely tens of thousand) > warnings/errors. I have started small over on the project we are using checkstyle with, by introducing JavaDoc checkstyle checks only. Even that introduced several thousand issues that are just now getting cleaned up. After these are cleaned up, we'll introduce some more checks. > > This said I'm in favour of trying to do this, even if it's only a "best > practice".
I think the AC should be mindful of too many recommendations -- especially to something as religious as code style. I doubt you could get a consensus on something as simple as "brackets for single line blocks". The best advice might be "everyone *should* use a code formatter and format on save". As for build time checks with a threshold, this seems rather arbitrary. Inevitably someone will remove some code right before a release (to fix a show stopper) and suddenly their builds will fail because more than xx% is now not formatted properly.
Looks like we all agree that + Having established formatting rules is good (whatever they look like) + Mechanically enforcing the rules is good (project settings) - Changing / adding the rules on an established codebase is a problem Looks like it would be a good thing to help new projects setup their rules / builds right from the start. Though I'm not sure how often we get completely new code at Eclipse. Having something to tweak (or turn off if need be) is better than having nothing.
(In reply to comment #9) > Looks like we all agree that > + Having established formatting rules is good (whatever they look like) > + Mechanically enforcing the rules is good (project settings) > - Changing / adding the rules on an established codebase is a problem > > Looks like it would be a good thing to help new projects setup their rules / > builds right from the start. Though I'm not sure how often we get completely > new code at Eclipse. Having something to tweak (or turn off if need be) is > better than having nothing. Any project that wants to run the current FindBugs rules and is using maven can now inherit the eclipse-parent pom. There is an analysis profile that can be used to run FindBugs against the projects code. http://wiki.eclipse.org/Maven/Parent_POM Checkstyle can also be run using the default Sun profile for checkstyle.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
As part of 565280 we have a new spotbugs profile (merged) and a pmd and checkstyle profile are waiting as gerrit in https://git.eclipse.org/r/c/platform/eclipse.platform.releng.aggregator/+/169736 https://git.eclipse.org/r/c/platform/eclipse.platform.releng.aggregator/+/169740 for review. For jdt there is a separate jenkins job to make use of it at https://ci.eclipse.org/jdt/job/eclipse.jdt.ui-SpotBugs/ Should it be possible to merge the gerrits we can close this one, can't we?
This issue has been migrated to https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/141.
