exercism / java

Exercism exercises in Java.
https://exercism.org/tracks/java
MIT License
697 stars 680 forks source link

Add Linting to the Java Track #1006

Closed masters3d closed 6 years ago

masters3d commented 7 years ago

I would be interested in seeing linting support as part of the build process.

I know Android provides some of these facilities. https://developer.android.com/studio/write/lint.html

I am not familiar with lint-mojo but it also seems to be an option. https://simpligility.github.io/android-maven-plugin/lint-mojo.html

I'd be happy to help implement this but I was hoping for some feedback.

masters3d commented 7 years ago

Here are other options: https://pmd.github.io/

Built in? https://docs.oracle.com/javase/8/docs/technotes/tools/unix/javac.html#BHCEECJF

-Xlint:all  
Enables all recommended warnings. In this release, enabling all available warnings is recommended.

Sample PR of somebody adding multiple Java Linters. I do not think we need all of these but maybe we would stick with the google code style?

https://github.com/commercetools/commercetools-sync-java/pull/32/files

I guess the goal would be to run the style check on the CI.

We already have solutions to run it locally:

https://github.com/google/styleguide/blob/gh-pages/eclipse-java-google-style.xml https://github.com/google/styleguide/blob/gh-pages/intellij-java-google-style.xml

masters3d commented 7 years ago

https://github.com/checkstyle/checkstyle https://github.com/jacoco/jacoco https://github.com/spotbugs/spotbugs https://github.com/pmd/pmd

Which one of these have you used before?

stkent commented 6 years ago

Good thought!

I've used all of the above before (well, FindBugs in place of SpotBugs) and actually help maintain a Gradle plugin that simplifies applying several different checkers to Java (soon: +Kotlin) projects: https://github.com/btkelly/gnag/. It could work well here. My concern would be that linting would push journey-test times back up towards the Travis timeout limit. We could open a PR and find out!

stkent commented 6 years ago

That plugin also has built-in PR commenting for feedback.

sjwarner-bp commented 6 years ago

I was also thinking about this, as I'd recently contributed to the Python track and found linting to be a part of their process.

I haven't used any Java linters before, but I think it could be a nice way to enforce consistency across the track. I agree with @stkent that this could cause Travis builds to take too long, but we won't know if we don't try it! :slightly_smiling_face:

On the subject of Travis, have we looked in to why the build failed in #1004 ? This might be the wrong place to discuss it, but should this be opened as an issue too?

stkent commented 6 years ago

CI could use a look at, for sure. I'm not sure master has been building lately (the newest builds I can see are 4 months old?) Same on the Kotlin track.

masters3d commented 6 years ago

Has anybody tried this: https://www.sonarqube.org https://sonarcloud.io/projects https://hub.docker.com/_/sonarqube/

Is free for open source.

Smarticles101 commented 6 years ago

Oh @stkent I know you've mentioned master branch on building on travis before and I remembered a discussion here. This comment by @kytrinyx is why I believe. Notice "Build branch updates" is turned off.

sjwarner-bp commented 6 years ago

I've only just seen this comment @Smarticles101 - that's a good catch! I see that setting is still turned off. Is there any reason for this @exercism/java? If not, I'm happy to flick the switch and see what happens.

As for linting, discussion here seems to have stalled - are there any preferences on a choice of linter?

stkent commented 6 years ago

Would that double-build PR branches?

sjwarner-bp commented 6 years ago

There is a second option for 'Build pull request updates'. If it did double-build PR branches, we could turn this one off? Do you think that would solve this issue? 🙂 I'm not too experienced with Travis but would love to learn!

stkent commented 6 years ago

It might! I think that would build every branch on push, not just on PR open/update, so we might still find it heavier, but it could be a decent solution. We can always try and revert, so go for it if you're ready!

sjwarner-bp commented 6 years ago

Sure thing - I'll enable "Build branch updates" and disable "Build pull request updates".

If this causes any issues, please anyone @exercism/java feel free to revert as I might not be around! 🙂

Smarticles101 commented 6 years ago

This exercise has long gone stale, what can we do to fix it? Is it still relevant or desired? Thoughts @exercism/java?

FridaTveit commented 6 years ago

I think adding linting would be useful for the track, however I haven't had time to look into it yet. I'm happy with any of the following options:

Or any other suggestions 🙂 What do you think @Smarticles101, @sjwarner-bp? 🙂

sjwarner-bp commented 6 years ago

I'd be happy to see it introduced, but not had time to deep-dive in yet...

I'm happy to keep it open for the moment - maybe as part of Hacktoberfest we will have some extra resource?

Smarticles101 commented 6 years ago

@sjwarner-bp good thought! I'll add the hacktoberfest label to this issue.

sjwarner-bp commented 6 years ago

I think it would be worth making a Hacktoberfest mega-thread issue in our repo so that we maintainers can work out how to make the most of this year's event, and ensure there is enough work to go around for all the new contributors :slightly_smiling_face: What do you think @Smarticles101 @FridaTveit

FridaTveit commented 6 years ago

Sounds like a good idea @sjwarner-bp 🙂

mirkoperillo commented 6 years ago

Hi,

I wanna try to help on this, but I need some more clear instructions about

FridaTveit commented 6 years ago

That's great that you want to help @mirkoperillo! 😄

I'm afaid we don't have very specific instructions at the moment. This issue is just to investigate adding linting to this repository, using something like for example CheckStyle. I hope that helps! Feel free to look into it if you want and let us know if you run into any issues! 🙂

mirkoperillo commented 6 years ago

ok, I will investigate a little bit

FridaTveit commented 6 years ago

I think this has been fixed by PR #1593. Thanks for all your great work @mirkoperillo! 😄