accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Drob <mad...@cloudera.com>
Subject Re: moving rat to a profile?
Date Thu, 19 Jun 2014 15:42:16 GMT
I hope you mean verify the output of rat:check, and not run "mvn verify" as
a pre-commit hook.


On Thu, Jun 19, 2014 at 10:38 AM, Bill Havanki <bhavanki@clouderagovt.com>
wrote:

> Ooh, had another thought. We can probably make the rat plugin run under a
> pre-commit hook [1]. That way, while you're actively developing, the rat
> plugin need not get in the way, but it still serves as a gatekeeper before
> you can commit.
>
> Git also allows for hooks around git am, so committers can invoke rat then
> to ensure contributed patches have licenses. That would be useful in case a
> contributor never commits locally, for example (or disables the pre-commit
> hook locally :) ).
>
> So, specifically, elements for this option:
> * by default, either do not run rat or run it with ignoreErrors=true
> * set pre-commit hook to run rat:check and verify
> * set pre-applypatch hook to also run rat:check and verify
>
> [1] http://git-scm.com/book/en/Customizing-Git-Git-Hooks
>
>
> On Tue, Jun 17, 2014 at 5:11 PM, Alex Moundalexis <alexm@clouderagovt.com>
> wrote:
>
> > I like this plan.
> >
> > * doesn't discourage new contributors
> > * provides information for those who want to dig deeper
> >
> > On Tue, Jun 17, 2014 at 5:04 PM, Bill Havanki <bhavanki@clouderagovt.com
> >
> > wrote:
> >
> > > It seems like a middle way would be:
> > >
> > > * always run the rat plugin
> > > * configure it by default with ignoreErrors=true
> > > * let committers / Jenkins / release managers et al. explicitly set
> > > rat.ignoreErrors=false (in MAVEN_OPTS or wherever)
> > >
> > > By default, the plugin will warn about files lacking a license, but
> will
> > > continue the build. Contributors are exposed to the check but not
> > > constrained by it. Example:
> > >
> > > ---
> > > [INFO] Rat check: Summary of files. Unapproved: 1 unknown: 1
> generated: 0
> > > approved: 187 licence.
> > > [WARNING] Rat check: 1 files with unapproved licenses. See RAT report
> in:
> > > /Users/bhavanki/dev/accumulo/server/base/target/rat.txt
> > > ---
> > >
> > > Any entity that should enforce licenses then needs to set the
> > ignoreErrors
> > > flag to false. This can be part of committer onboarding.
> > >
> > > Bill
> > >
> > >
> > > On Tue, Jun 17, 2014 at 4:59 PM, Josh Elser <josh.elser@gmail.com>
> > wrote:
> > >
> > > >
> > > >
> > > > On 6/17/14, 1:47 PM, Alex Moundalexis wrote:
> > > >
> > > >> This kind of response is hardly conducive to prospective
> contributors.
> > > >>
> > > >> We should consider ourselves lucky whenever a contributor provides
a
> > > >> patch,
> > > >> let alone runs a build. Expecting a new contributor be fully aware
> of
> > > the
> > > >> Apache licensing details isn't realistic, much less being aware of
> the
> > > >> arguments concerning Rat; if the ignoreErrors argument is TheWay,
it
> > > ought
> > > >> to be mentioned prominently in the source documentation [1], but I
> > don't
> > > >> think that's correct either...
> > > >>
> > > >> I don't want to encourage contributors to skip the build. I want
> > > >> contributors to be aware of the licensing requirements, but not at
> the
> > > >> expense of providing otherwise-viable patches. I'd recommend
> relaxing
> > > the
> > > >> Rat checks for contributors, and making it a required part of the
> > > profile
> > > >> for automated Jenkins builds and during the release process.
> > > >>
> > > >> The onus should be on the committers to ensure that all of the
> > licensing
> > > >> is
> > > >> in place before the release, but preferably long before that point
> by
> > > >> guiding the contributor to make the necessary license additions
> before
> > > the
> > > >> commit.
> > > >>
> > > >
> > > > This is an important thing to remember. The point of shepherding
> > > > contributors is to eventually get them to committer status, at which
> > > point
> > > > they'll be personally responsible for these things. While we
> definitely
> > > > don't want to be to abrasive initially that they get fed up and go
> > away,
> > > we
> > > > can't fully insulate from the necessary either.
> > > >
> > > >
> > > >
> > > >> I've been told to correct whitespace at the end of a line before and
> > to
> > > >> re-submit a patch, seems trivial to address missing licensing files
> in
> > > the
> > > >> same way.
> > > >>
> > > >> [1] https://accumulo.apache.org/source.html
> > > >>
> > > >>
> > >
> > >
> > > --
> > > // Bill Havanki
> > > // Solutions Architect, Cloudera Govt Solutions
> > > // 443.686.9283
> > >
> >
>
>
>
> --
> // Bill Havanki
> // Solutions Architect, Cloudera Govt Solutions
> // 443.686.9283
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message