Where to integrate checkstyle into gerrit
881 views
Skip to first unread message
Joel
Mar 28, 2011, 6:51:24 PM3/28/11
to Repo and Gerrit Discussion
Hi,
At work we're looking at integrating checkstyle into gerrit
http://checkstyle.sourceforge.net/, what class in gerrit would I start
looking at to do this?
I have made minor customisations to gerrit before, so it's not
completely brand new to me.
Cheers,
-Joel
At work we're looking at integrating checkstyle into gerrit
http://checkstyle.sourceforge.net/, what class in gerrit would I start
looking at to do this?
I have made minor customisations to gerrit before, so it's not
completely brand new to me.
Cheers,
-Joel
Ishaaq Chandy
Mar 28, 2011, 8:27:13 PM3/28/11
to Joel, Repo and Gerrit Discussion
Why would you want to do that? Gerrit isn't a CI tool.
We use checkstyle at work too, and a boatload of other tools as well for analysing and testing our code. We use a Hudson/Jenkins cluster to run each new commit through a barrage of these checks. The Hudson machines have ssh keys that we've registered with Gerrit so as to make them reviewers. For a commit to be successfully merged into Gerrit we require that a human reviewer gives the commit CRVW=+2 and that Hudson gives it VRIF=+1
This scheme works very well for us as we can then guarantee that code can only be merged into the remote branches on Gerrit after they have been thoroughly checked.
Ishaaq
We use checkstyle at work too, and a boatload of other tools as well for analysing and testing our code. We use a Hudson/Jenkins cluster to run each new commit through a barrage of these checks. The Hudson machines have ssh keys that we've registered with Gerrit so as to make them reviewers. For a commit to be successfully merged into Gerrit we require that a human reviewer gives the commit CRVW=+2 and that Hudson gives it VRIF=+1
This scheme works very well for us as we can then guarantee that code can only be merged into the remote branches on Gerrit after they have been thoroughly checked.
Ishaaq
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en
Joel
Mar 29, 2011, 1:25:17 AM3/29/11
to Repo and Gerrit Discussion
Hi Ishaaq,
I agree Gerrit isn't a CI tool, and we already have Hudson setup with
the Gerrit Trigger doing the VRIF=+1.
However we don't plugin checkstyle into hudson, because we already
have too many violations because it's an old codebase and we can't
expect every commit to fix unrelated violations in the same class.
So the idea is to overlay checkstyle information in gerrit to aid in
code review, like how grepcode does it:
http://grepcode.com/file/repo1.maven.org/maven2/org.testng/testng/5.14.6/org/testng/internal/annotations/TestOrConfiguration.java#TestOrConfiguration.setEnabled%28boolean%29
If you see the little bug icons on the left of the code, you can hover
over it and it tells you the violation.
I imagine this shouldn't be all that difficult to implement in gerrit,
but I'm just not sure where to start from.
Cheers,
-Joel
On Mar 29, 11:27 am, Ishaaq Chandy <ish...@gmail.com> wrote:
> Why would you want to do that? Gerrit isn't a CI tool.
>
> We use checkstyle at work too, and a boatload of other tools as well for
> analysing and testing our code. We use a Hudson/Jenkins cluster to run each
> new commit through a barrage of these checks. The Hudson machines have ssh
> keys that we've registered with Gerrit so as to make them reviewers. For a
> commit to be successfully merged into Gerrit we require that a human
> reviewer gives the commit CRVW=+2 and that Hudson gives it VRIF=+1
>
> This scheme works very well for us as we can then guarantee that code can
> only be merged into the remote branches on Gerrit after they have been
> thoroughly checked.
>
> Ishaaq
>
I agree Gerrit isn't a CI tool, and we already have Hudson setup with
the Gerrit Trigger doing the VRIF=+1.
However we don't plugin checkstyle into hudson, because we already
have too many violations because it's an old codebase and we can't
expect every commit to fix unrelated violations in the same class.
So the idea is to overlay checkstyle information in gerrit to aid in
code review, like how grepcode does it:
http://grepcode.com/file/repo1.maven.org/maven2/org.testng/testng/5.14.6/org/testng/internal/annotations/TestOrConfiguration.java#TestOrConfiguration.setEnabled%28boolean%29
If you see the little bug icons on the left of the code, you can hover
over it and it tells you the violation.
I imagine this shouldn't be all that difficult to implement in gerrit,
but I'm just not sure where to start from.
Cheers,
-Joel
On Mar 29, 11:27 am, Ishaaq Chandy <ish...@gmail.com> wrote:
> Why would you want to do that? Gerrit isn't a CI tool.
>
> We use checkstyle at work too, and a boatload of other tools as well for
> analysing and testing our code. We use a Hudson/Jenkins cluster to run each
> new commit through a barrage of these checks. The Hudson machines have ssh
> keys that we've registered with Gerrit so as to make them reviewers. For a
> commit to be successfully merged into Gerrit we require that a human
> reviewer gives the commit CRVW=+2 and that Hudson gives it VRIF=+1
>
> This scheme works very well for us as we can then guarantee that code can
> only be merged into the remote branches on Gerrit after they have been
> thoroughly checked.
>
> Ishaaq
>
Ted
Mar 29, 2011, 1:36:54 AM3/29/11
to Joel, Repo and Gerrit Discussion
I'm also using gerrit + hudson + checkstyle. I also question the desire for gerrit & checkstyle integration.
There are other ways around having too many violations... the project I'm on right now tops out at over 100,000 violations and that's with most of the checking turned off. What I did instead was I wrote a custom regex check which stored existing violations into a xml file. This helps prevent new violations from being commit as those would thrown an error in the build, but it would allow existing violations to continue to exist.
With that, the normal chain of events of gerrit & hudson build failures and -1 verifies, works as normal.--
Ted.
There are other ways around having too many violations... the project I'm on right now tops out at over 100,000 violations and that's with most of the checking turned off. What I did instead was I wrote a custom regex check which stored existing violations into a xml file. This helps prevent new violations from being commit as those would thrown an error in the build, but it would allow existing violations to continue to exist.
With that, the normal chain of events of gerrit & hudson build failures and -1 verifies, works as normal.
Ted.
Joel
Mar 29, 2011, 1:51:19 AM3/29/11
to Repo and Gerrit Discussion
Hi Ted,
That sounds interesting. Did you do something like this with the
SupressionsFilter? http://stackoverflow.com/questions/5269131/how-to-define-scope-while-running-checkstyles
Either way we're still keen to add this checkstyle overlay to gerrit
at some point, but I think that ignoring existing violations could be
a first good step.
Cheers,
-Joel
On Mar 29, 4:36 pm, Ted <r6squee...@gmail.com> wrote:
> I'm also using gerrit + hudson + checkstyle. I also question the desire for
> gerrit & checkstyle integration.
>
> There are other ways around having too many violations... the project I'm on
> right now tops out at over 100,000 violations and that's with most of the
> checking turned off. What I did instead was I wrote a custom regex check
> which stored existing violations into a xml file. This helps prevent new
> violations from being commit as those would thrown an error in the build,
> but it would allow existing violations to continue to exist.
>
> With that, the normal chain of events of gerrit & hudson build failures and
> -1 verifies, works as normal.
>
>
>
>
>
>
>
>
>
> On Tue, Mar 29, 2011 at 4:25 PM, Joel <joel.pear...@gmail.com> wrote:
> > Hi Ishaaq,
>
> > I agree Gerrit isn't a CI tool, and we already have Hudson setup with
> > the Gerrit Trigger doing the VRIF=+1.
>
> > However we don't plugin checkstyle into hudson, because we already
> > have too many violations because it's an old codebase and we can't
> > expect every commit to fix unrelated violations in the same class.
>
> > So the idea is to overlay checkstyle information in gerrit to aid in
> > code review, like how grepcode does it:
>
> >http://grepcode.com/file/repo1.maven.org/maven2/org.testng/testng/5.1...
That sounds interesting. Did you do something like this with the
SupressionsFilter? http://stackoverflow.com/questions/5269131/how-to-define-scope-while-running-checkstyles
Either way we're still keen to add this checkstyle overlay to gerrit
at some point, but I think that ignoring existing violations could be
a first good step.
Cheers,
-Joel
On Mar 29, 4:36 pm, Ted <r6squee...@gmail.com> wrote:
> I'm also using gerrit + hudson + checkstyle. I also question the desire for
> gerrit & checkstyle integration.
>
> There are other ways around having too many violations... the project I'm on
> right now tops out at over 100,000 violations and that's with most of the
> checking turned off. What I did instead was I wrote a custom regex check
> which stored existing violations into a xml file. This helps prevent new
> violations from being commit as those would thrown an error in the build,
> but it would allow existing violations to continue to exist.
>
> With that, the normal chain of events of gerrit & hudson build failures and
> -1 verifies, works as normal.
>
>
>
>
>
>
>
>
>
> On Tue, Mar 29, 2011 at 4:25 PM, Joel <joel.pear...@gmail.com> wrote:
> > Hi Ishaaq,
>
> > I agree Gerrit isn't a CI tool, and we already have Hudson setup with
> > the Gerrit Trigger doing the VRIF=+1.
>
> > However we don't plugin checkstyle into hudson, because we already
> > have too many violations because it's an old codebase and we can't
> > expect every commit to fix unrelated violations in the same class.
>
> > So the idea is to overlay checkstyle information in gerrit to aid in
> > code review, like how grepcode does it:
>
Shane Mc Cormack
Mar 29, 2011, 5:24:21 AM3/29/11
to Repo and Gerrit Discussion
Hi,
Rather than added it into gerrit directly (remembering that gerrit does more than just java), you could instead add an ssh command to allow adding line-level comments to files in review, then make a separate hook that ran check style and added the violations in that way.
- Shane
Rather than added it into gerrit directly (remembering that gerrit does more than just java), you could instead add an ssh command to allow adding line-level comments to files in review, then make a separate hook that ran check style and added the violations in that way.
- Shane
Nasser Grainawi
Mar 29, 2011, 11:00:59 AM3/29/11
to Shane Mc Cormack, Repo and Gerrit Discussion
Yes, I think this would very much be the preferred way to handle adding this kind of integration (short of adding a full-on plugin interface to Gerrit). When we've discussed doing "static analysis"-type automated verification on pending Gerrit changes Shawn has indicated line-level comments would be the way to go [1].
Nasser
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Brad Larson
Mar 29, 2011, 3:31:42 PM3/29/11
to Repo and Gerrit Discussion
On Mar 29, 10:00 am, Nasser Grainawi <nas...@codeaurora.org> wrote:
> Yes, I think this would very much be the preferred way to handle adding this kind of integration (short of adding a full-on plugin interface to Gerrit). When we've discussed doing "static analysis"-type automated verification on pending Gerrit changes Shawn has indicated line-level comments would be the way to go [1].
integration once this is in place.
>
> Nasser
>
> [1]http://code.google.com/p/gerrit/issues/detail?id=602
>
> On Mar 29, 2011, at 3:24 AM, Shane Mc Cormack wrote:
>
>
>
>
>
>
>
>
>
> > Hi,
>
> > Rather than added it into gerrit directly (remembering that gerrit does more than just java), you could instead add an ssh command to allow adding line-level comments to files in review, then make a separate hook that ran check style and added the violations in that way.
>
> > - Shane
>
> > On Tue, Mar 29, 2011 at 06:51, Joel <joel.pear...@gmail.com> wrote:
> > Hi Ted,
>
> > That sounds interesting. Did you do something like this with the
> > SupressionsFilter?http://stackoverflow.com/questions/5269131/how-to-define-scope-while-...
> > Hi Ted,
>
> > That sounds interesting. Did you do something like this with the
Steffen Gebert
Apr 4, 2011, 3:42:06 AM4/4/11
to Repo and Gerrit Discussion
> > When we've discussed doing "static analysis"-type automated verification on pending Gerrit changes Shawn has indicated line-level comments would be the way to go [1].
>
> +1, I think we could see some really cool CI/Buildbot<->Gerrit
> integration once this is in place.
+1 from me, too, for inline comments via SSH!
>
> +1, I think we could see some really cool CI/Buildbot<->Gerrit
> integration once this is in place.
We'd also like to have checkstyle integrated in jenkins.
Steffen
Reply all
Reply to author
Forward
0 new messages