Skip to content
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

6070 reviewdog checkstyle #7911

Merged
merged 1 commit into from
May 28, 2021

Conversation

poikilotherm
Copy link
Contributor

What this PR does / why we need it:
Now that #7801 has been merged and #6070 is resolved, we should make sure that no one reintroduces using the Apache Commons Lang v2 library by misusing a transitive dependency.

This PR will create a Github Action using @reviewdog to bark on any NEW checkstyle violations (not the existing), which includes making imports from the Java package invalid.

Which issue(s) this PR closes:
Relates to #6070

Special notes for your reviewer:
See #7886 for a demo (although it does not yet use inline review comments as used from a fork).

Suggestions on how to test this:
See the demo ;-)

Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Nope.

Is there a release notes update needed for this change?:
Nope.

Additional documentation:
We can enable more rules in the future. It would be good to check for code styling on new PRs and not rely on humans for that chore task.

@poikilotherm poikilotherm added Type: Feature a feature request Component: Code Infrastructure formerly "Feature: Code Infrastructure" labels May 28, 2021
@poikilotherm poikilotherm force-pushed the 6070-reviewdog-checkstyle branch from ab1a391 to 749a33e Compare May 28, 2021 13:41
@coveralls
Copy link

coveralls commented May 28, 2021

Coverage Status

Coverage remained the same at 19.324% when pulling 749a33e on poikilotherm:6070-reviewdog-checkstyle into 52fee75 on IQSS:develop.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems great. It's using our existing checkstyle.xml config that we document at https://guides.dataverse.org/en/5.5/developers/coding-style.html#checking-your-formatting-with-checkstyle

I doubt developers are actively running mvn checkstyle:checkstyle before running a pull request. (I'm not.) This automates that process.

After we get comfortable with it we might want to add some extra docs somewhere to explain to people making pull requests that a dog might bark at them but that it's ok and that we're still friendly to contributors. 😄 🐶

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Code Infrastructure formerly "Feature: Code Infrastructure" Type: Feature a feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants