hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jan Hentschel (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-19819) Decide the place of Checkstyle in build flow
Date Fri, 26 Jan 2018 20:20:00 GMT

    [ https://issues.apache.org/jira/browse/HBASE-19819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16341533#comment-16341533

Jan Hentschel commented on HBASE-19819:

[~stack] My first intention in HBASE-12521 was to make Checkstyle part of the build process,
meaning that it runs as part of {{mvn install}}. I started to do it on a module basis, introducing
Checkstyle in the build for every sanitized module with the ultimate goal to move it up to
the parent POM as soon as all modules have been sanitized. As a result of the discussion related
to the build problems [~appy] mentioned that making Checkstyle part of the build it would
nearly double the build time. As a compromise we discussed to make Checkstyle part of the
pre-commit build, which will fail on Checkstyle errors, and also put a note into the documentation
so that every developer should be aware of it.

I'm not sure if we can have a perfect solution. In my internal projects I like to have everything
necessary before committing be part of the build itself, so a developer can see right in his
local build if everything is fine. I see [~appy]s point, so we came up with the suggestion
mentioned above. For me the goal of this ticket to have some feedback from the "outside" and
come up with a solution we can proceed with.

> Decide the place of Checkstyle in build flow
> --------------------------------------------
>                 Key: HBASE-19819
>                 URL: https://issues.apache.org/jira/browse/HBASE-19819
>             Project: HBase
>          Issue Type: Improvement
>            Reporter: Appy
>            Priority: Major
> Ref: https://issues.apache.org/jira/browse/HBASE-19780
>  Main questions:
>  # Should checkstyle (CS) be part of {{mvn install}}. On master, mvn install (without
clean) takes ~3 min and {{mvn checkstyle:checkstyle}} takes ~2min.
>  I think the reason it's not part of default build might be - our project isn't clean,
and failing because of existing 10k CS issues is useless. Maybe there's no trivial way of
reporting just the new CS issues, and that's why we depend on QA (which gives just the diff)
for checkstyle?
>  # How to avoid regressions in modules which have been sanitized by [~Jan Hentschel].
Here's a suggestion building on his:
> ** Let's add a recommendation in documentation that run mvn checkstyle:check before submitting
patches since it'll catch CS violations in modules which are perfectly clean.
>  ** Add checkstyle:check as part of main pre-commit build. If there is any violation
in these clean modules (towards which you have put great effort), then the pre-commit will
fail also for the mvn install step, which is an important one. Thus, clean CK in these modules
become hard pre-commit requirement indirectly.
>  Let's put a note on dev@ proposing these changes.

This message was sent by Atlassian JIRA

View raw message