accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher <ctubb...@apache.org>
Subject Re: moving rat to a profile?
Date Thu, 19 Jun 2014 16:08:35 GMT
Well, to be clear rat:check executes a different plugin than the apache-rat
one we use. However, it'd be good to do "mvn validate", which will execute
the correct rat check and the enforcer plugin and other very basic checks.


--
Christopher L Tubbs II
http://gravatar.com/ctubbsii


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

> I do. :)
>
> "... to run rat:check and verify that it passes"
>
>
> On Thu, Jun 19, 2014 at 11:42 AM, Mike Drob <madrob@cloudera.com> wrote:
>
> > 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
> > >
> >
>
>
>
> --
> // Bill Havanki
> // Solutions Architect, Cloudera Govt Solutions
> // 443.686.9283
>

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