You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Christopher <ct...@apache.org> on 2014/07/11 00:09:06 UTC

Re: moving rat to a profile?

Since this came up in Review (https://reviews.apache.org/r/23391/), I'm
revisiting this thread.

In short, it seems there was some confusion on whether or not we had
reached consensus on enabling rat by default, with rat.ignoreErrors=true
set to default, and requiring any entities that should be responsible for
checking legal status of contributions to turn it in (set
rat.ignoreErrors=false for Jenkins, committers, etc.)

I think one problem is that there was not actually consensus. This was a
proposal made previously by Bill Havanki to do the above, after which we
agreed to minimally making changes to make "git clean -df" sufficient. But
there did not appear to be any consensus reached on disabling error
checking by default, as proposed. Billie had expressed concerns about
encouraging committers to not take care in checking the legal status of
their commits, and not only do I share those concerns, I'd also extend my
concerns to any contributor. This is simply something that everybody
contributing to ASF should be aware of. Committers have the extra
responsibility, but even contributors and potential committers should
understand the contribution requirements, the development process, and have
put in some minimal amount of thought about whether they have the ability
to contribute a particular piece of code legally.

Additionally, I'm concerned about who we'd be easing things for. There's
clearly an increased risk of commits/contributions without strict checking
for licenses (we've seen past commits with these errors... just do `git log
| grep -i license` to see that). So, we're accepting that risk by making
the easy route one that serves who, exactly? The ASF makes it quite clear
that SCM is for developers, not end users, and this is an SCM-specific
problem (specifically branch switching in git).

So, I do not think there is consensus. There are certainly outstanding
concerns for disabling the rat check by default. However, I think there is
room for further discussion and compromise, and I'd love to consider more
solutions (or discuss the merits of the proposed solutions). For instance,
one area of compromise I've conceded (as stated in the review) is that I
think it'd be a good idea to put the rat check in a profile which can be
optionally disabled, so that it doesn't take the 3 extra seconds to run,
which could impact some iterative testing (like running one IT at a time,
or doing automated `git bisect`).




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


On Thu, Jun 19, 2014 at 11:15 AM, Christopher <ct...@apache.org> wrote:

> Agreed. That's a minimum.
>
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>
>
> On Thu, Jun 19, 2014 at 10:54 AM, Bill Havanki <bh...@clouderagovt.com>
> wrote:
>
>> I've filed ACCUMULO-2927 to make 'git clean -df' sufficient. No matter how
>> we decide about the rat plugin, I think not requiring -x is a worthwhile
>> goal.
>>
>>
>> On Tue, Jun 17, 2014 at 5:43 PM, Christopher <ct...@apache.org> wrote:
>>
>> > I agree that instructing users to use this option to modify the build
>> isn't
>> > acceptable and I wouldn't recommend this as a response to users... I was
>> > only stating this as a fact, to point out that a special profile on by
>> > default with an option to disable isn't needed, since that's the current
>> > behavior.
>> >
>> > I'm more interested in the targeted .gitignore with the recommended "git
>> > clean -df" option without -x. This helps contributors understand build
>> > tools, makes them aware of the differences between branches, and doesn't
>> > hide problems introduced by switching branches in an obscure error, all
>> > without blowing away their IDE build files. (though switching branches
>> > often warrants blowing these IDE files away anyway, since different
>> modules
>> > in different branches will be problematic for most IDEs).
>> >
>> >
>> > --
>> > Christopher L Tubbs II
>> > http://gravatar.com/ctubbsii
>> >
>> >
>> > On Tue, Jun 17, 2014 at 4:47 PM, Alex Moundalexis <
>> alexm@clouderagovt.com>
>> > 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.
>> > >
>> > > 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
>> > >
>> > > On Tue, Jun 17, 2014 at 3:15 PM, Christopher <ct...@apache.org>
>> > wrote:
>> > >
>> > > > There's already a way to skip it for those who don't understand why
>> its
>> > > > failing and are incapable/unwilling to troubleshoot:
>> > > > -Drat.ignoreErrors=true
>> > > >
>> > > >
>> > > > --
>> > > > Christopher L Tubbs II
>> > > > http://gravatar.com/ctubbsii
>> > > >
>> > > >
>> > > > On Tue, Jun 17, 2014 at 3:09 PM, Billie Rinaldi <
>> > > billie.rinaldi@gmail.com>
>> > > > wrote:
>> > > >
>> > > > > I'm not thrilled about turning it off by default.  How about
>> putting
>> > it
>> > > > in
>> > > > > a profile that would be enabled by default, but could be disabled
>> > with
>> > > a
>> > > > > flag for those who don't understand why it's failing?
>> > > > >
>> > > > >
>> > > > > On Tue, Jun 17, 2014 at 11:44 AM, Sean Busbey <
>> busbey@cloudera.com>
>> > > > wrote:
>> > > > >
>> > > > > > I've had a few different new-to-Accumulo contributors recently
>> run
>> > > into
>> > > > > the
>> > > > > > issue of Rat failing the build after changing branches.
>> > > > > >
>> > > > > > I know we already have a warning about this[1], but AFAICT it's
>> > over
>> > > > the
>> > > > > > threshold for consumable information.
>> > > > > >
>> > > > > > Even after pointing people to the warning, the existing
>> workaround
>> > > > > tripped
>> > > > > > up atleast one of them. Despite the warning about using "git
>> > clean,"
>> > > > the
>> > > > > > destruction of their local IDE changes were surprising.
>> > > > > >
>> > > > > > For contributions to Accumulo that aren't coming from
>> committers,
>> > the
>> > > > Rat
>> > > > > > plugin seems much more likely to give a false positive than to
>> > catch
>> > > an
>> > > > > > error. Additionally, whatever committer is reviewing the
>> > contribution
>> > > > > > should be checking for license compliance anyways.
>> > > > > >
>> > > > > > In the interests of reducing the surprise for new contributors,
>> I'd
>> > > > like
>> > > > > to
>> > > > > > move our use of Rat to a profile that is only default enabled
>> > during
>> > > a
>> > > > > > release run.
>> > > > > >
>> > > > > > The profile would still let those who want rat to run on every
>> > build
>> > > to
>> > > > > > enable it and we could update the guide for handling new
>> > > contributions
>> > > > to
>> > > > > > say committers should enable the rat profile to help guard
>> against
>> > > > > errors.
>> > > > > >
>> > > > > > Any objections?
>> > > > > >
>> > > > > > [1]: http://accumulo.apache.org/source.html#running-a-build
>> > > > > >
>> > > > > > --
>> > > > > > Sean
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>>
>>
>> --
>> // Bill Havanki
>> // Solutions Architect, Cloudera Govt Solutions
>> // 443.686.9283
>>
>
>

Re: moving rat to a profile?

Posted by Christopher <ct...@apache.org>.
Apologies for the length. Responses inline. In short, I'd retract my veto
on the code change subject to the results of a majority vote, as the
quickest way to make progress.

On Fri, Jul 11, 2014 at 8:59 AM, Sean Busbey <bu...@cloudera.com> wrote:

> All due respect Christopher, you're a developer with many years of
> experience who's worked with this code base for a decent amount of time;
> that you don't find the error message problematic has very little bearing
> on the fact that new contributors have (and have said as much multiple
> times).
>

A couple of things about this point. First, there seems to be the
implication that because I've been working with Accumulo for some time that
I'm incapable of empathizing with the frustrations of new contributors.
You've made this implication before, and I find it a bit antagonistic, and
it's simply not true. My stated views are as they are, in full knowledge
of, and empathy for, the frustrations that new contributors may encounter.

Second, there does not appear to be any reason to believe that any one of
us has more or less capacity to consider these consequences than another
one of us, and I think it would be best to avoid making arguments based on
the premise that one of us does. After all, one could just as easily make
the argument that experience with the project also provides insight into
the experiences of a greater number of new contributors' experiences. I'd
rather not devolve our arguments to such a state. I'd rather we focus our
arguments on their merits, rather than the authority (either by virtue of
experience or lack thereof) of the person making the argument and
implications of the capacity the other has to have a fully informed
opinion. Otherwise, we're essentially suggesting "your opinion doesn't
count as much as mine", and I don't think such suggestions are beneficial
to the progress of the argument.


> I'm not saying that new contributors shouldn't think about licensing at
> all. I'm saying that we should ease them into it, similar to how we ease
> them into everything else (e.g. integration tests).
>

I don't mind easing them into it. I just think that the current state is
sufficient easing, whereas any further easing would begin to trade off too
much.

I don't believe we are at a point of irreconcilable differences.
> Specifically, I haven't heard any technical justification that we are at an
> increased risk of improperly licensed files getting into the code base
> provided that committers follow the proposed changed instructions to check
> contributions for rat failures (esp if they follow the instructions to
> default all of their builds to fail on rat errors). Do you believe that our
> committers will not follow these instructions? If so, would a git hook that
> did the enforcement suffice?
>
-Sean
>
>
The irreconcilable difference I think is not a completely dichotomous one.
I just think it's just a matter of scale: you find the current state
insufficiently easy and that further easing would not lose too much,
whereas I think the current state is sufficiently easy and that further
easing would lose too much. The difference is slight, I'm sure, but it does
appear to be irreconcilable (at least, irreconcilable in a timely manner,
without a vote).

The key part is "provided that committers follow the proposed changed
instructions..."; That itself is increased risk, because we are now relying
on committers to follow increased steps, when we can objectively see the
history and determine that committers are just as likely to inadvertently
overlook these responsibilities as anybody. It's not that I don't believe
our committers can follow those instructions... it's just a simple
recognition that they haven't always done the best job in the past at doing
what is necessary to check licenses (myself included).

I think a git hook would help, yes, but I'd also want contributors thinking
about these things from the beginning, and I think we trade off some
conscious consideration on the part of contributors if we make the default
build too easy. I think contributors should begin thinking about these
things before they ever upload a patch to JIRA or submit a pull request,
and the current default settings is the only thing that has been considered
that accomplishes that, so that we do not train anybody to be lazy when it
comes to license checking. I really think the user should be confronted
with the results of a license check as soon as possible. Now, I also think
that confrontation could be made more verbose... and I fully support and
encourage anybody willing to tackle the RAT tickets that would do that (I
may even take them up myself, if I ever have time).

I'd be willing to retract my veto on the code change, subject to the
outcome of a majority vote. I don't know that anybody else has formally
vetoed the code change, so if I'm the only one, a majority vote would seem
to be the quickest way to resolve this and put it behind us, if you want to
call one up.

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




>
>
> On Fri, Jul 11, 2014 at 7:24 AM, Christopher <ct...@apache.org> wrote:
>
> > I'd like to point out that there's a few upstream JIRAs related to this
> > conversation that, if resolved, might address all concerns in this
> thread:
> >
> > https://issues.apache.org/jira/browse/RAT-61
> > https://issues.apache.org/jira/browse/RAT-157
> > https://issues.apache.org/jira/browse/RAT-164
> >
> > To the points made, I really don't find the error message to be that
> > problematic for new contributors, especially now that `git status`
> > prominently shows the ignored files left behind from a build in a
> different
> > branch. It tells you precisely what the problem is, and which file to
> look
> > in to to see the details. The bare minimum thing a contributor can do is
> > read the error message, after all. And, doing so will begin getting them
> > into the mindset of understanding the licensing, which is really a
> > prerequisite for contributing.
> >
> > I empathize with the concerns enumerated about the error being related to
> > something other than what the user did... but the fact is, it's not
> > unrelated to what the user did. It's related to the fact that they are
> > using the SCM, building, then switching branches, then building again
> > without cleaning up the previous build. This would be equivalent to
> > unpacking a tarball and running `make`, then unpacking a different
> tarball
> > over top of the first and running `make install`. It's not unrelated to
> the
> > contributor's actions. However, it is true that they may not understand
> the
> > quirks of maven, git, and the rat plugin, to understand how it is related
> > to what they did, and with that I share the expressed frustration, but am
> > also more than willing to help any contributors fill this gap in their
> > understanding (as I'm sure we all are). I further doubt that this has
> > actually caused any possible contributor to be turned away, without so
> much
> > as a question in IRC or on the mailing lists.
> >
> > At this point, I think there's probably an irreconcilable difference in
> > views regarding the default action of this plugin. So, perhaps it might
> be
> > necessary to simply vote on the issue.
> >
> >
> >
> > --
> > Christopher L Tubbs II
> > http://gravatar.com/ctubbsii
> >
> >
> > On Fri, Jul 11, 2014 at 7:07 AM, Sean Busbey <bu...@cloudera.com>
> wrote:
> >
> > > Changes to ease ignoring the rat report (but leaving it enabled to fail
> > the
> > > build by default) only help established contributors. Our changes to
> make
> > > clean up of trash easier similarly only help more established
> > contributors.
> > >
> > > We have a duty to help grow the community. A part of that is committers
> > > helping to ease new contributors into the requirements that they need
> to
> > > fulfill. We already know that having the rat plugin fail the build, in
> > > combination with it having no way to ignore trash from other branches,
> > is a
> > > stumbling block for new contributors.
> > >
> > > It is much better to have a committer guide a new contributor through
> > > something like a missing license header than it is to have that new
> > > contributor run into the current failure.
> > >
> > > * It's a failed build when they "haven't done anything yet" -
> something I
> > > use as a major red flag about new communities and I'm sure others do as
> > > well
> > > * It's a failure message that gives you 0 information until you dig
> into
> > > the report
> > > * In the most common case, that report tells you it's a problem
> unrelated
> > > to anything you did
> > > * It's a problem that isn't fixed with "mvn clean", so new users have
> to
> > > ask for guidance (presuming they don't give up)
> > >
> > > Provided that we have instructions on how committers should be checking
> > new
> > > contributions with the plugin combined with instructions for committers
> > to
> > > default the plugin to fail the build, what is the added risk?
> > >
> > > We'll have Jenkins as a fall back to keep us honest, just like we do to
> > > make sure committers are checking that things compile at all.
> > >
> > >
> > > On Thu, Jul 10, 2014 at 10:25 PM, Eric Newton <er...@gmail.com>
> > > wrote:
> > >
> > > > -1 for removing the rat plugin for default builds
> > > > +1 for a simple way to ignore it
> > > >
> > > > I find Christopher's reasoning sound.  I was guilty of committing
> half
> > > the
> > > > non-compliant files before the rat check was on by default.
> > > >
> > > > I wish the rat check was less annoying about build trash.
> > > > I wish the rat check produced error messages that didn't require me
> to
> > > > consult a report.
> > > > I wish the template mechanism in eclipse worked every time so source
> > > files
> > > > would always get the copyright header by default.
> > > >
> > > > But, automating the check, and moving it to an earlier point in the
> dev
> > > > cycle is a good thing.
> > > >
> > > > -Eric
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > On Thu, Jul 10, 2014 at 6:09 PM, Christopher <ct...@apache.org>
> > > wrote:
> > > >
> > > > > Since this came up in Review (https://reviews.apache.org/r/23391/
> ),
> > > I'm
> > > > > revisiting this thread.
> > > > >
> > > > > In short, it seems there was some confusion on whether or not we
> had
> > > > > reached consensus on enabling rat by default, with
> > > rat.ignoreErrors=true
> > > > > set to default, and requiring any entities that should be
> responsible
> > > for
> > > > > checking legal status of contributions to turn it in (set
> > > > > rat.ignoreErrors=false for Jenkins, committers, etc.)
> > > > >
> > > > > I think one problem is that there was not actually consensus. This
> > was
> > > a
> > > > > proposal made previously by Bill Havanki to do the above, after
> which
> > > we
> > > > > agreed to minimally making changes to make "git clean -df"
> > sufficient.
> > > > But
> > > > > there did not appear to be any consensus reached on disabling error
> > > > > checking by default, as proposed. Billie had expressed concerns
> about
> > > > > encouraging committers to not take care in checking the legal
> status
> > of
> > > > > their commits, and not only do I share those concerns, I'd also
> > extend
> > > my
> > > > > concerns to any contributor. This is simply something that
> everybody
> > > > > contributing to ASF should be aware of. Committers have the extra
> > > > > responsibility, but even contributors and potential committers
> should
> > > > > understand the contribution requirements, the development process,
> > and
> > > > have
> > > > > put in some minimal amount of thought about whether they have the
> > > ability
> > > > > to contribute a particular piece of code legally.
> > > > >
> > > > > Additionally, I'm concerned about who we'd be easing things for.
> > > There's
> > > > > clearly an increased risk of commits/contributions without strict
> > > > checking
> > > > > for licenses (we've seen past commits with these errors... just do
> > `git
> > > > log
> > > > > | grep -i license` to see that). So, we're accepting that risk by
> > > making
> > > > > the easy route one that serves who, exactly? The ASF makes it quite
> > > clear
> > > > > that SCM is for developers, not end users, and this is an
> > SCM-specific
> > > > > problem (specifically branch switching in git).
> > > > >
> > > > > So, I do not think there is consensus. There are certainly
> > outstanding
> > > > > concerns for disabling the rat check by default. However, I think
> > there
> > > > is
> > > > > room for further discussion and compromise, and I'd love to
> consider
> > > more
> > > > > solutions (or discuss the merits of the proposed solutions). For
> > > > instance,
> > > > > one area of compromise I've conceded (as stated in the review) is
> > that
> > > I
> > > > > think it'd be a good idea to put the rat check in a profile which
> can
> > > be
> > > > > optionally disabled, so that it doesn't take the 3 extra seconds to
> > > run,
> > > > > which could impact some iterative testing (like running one IT at a
> > > time,
> > > > > or doing automated `git bisect`).
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Christopher L Tubbs II
> > > > > http://gravatar.com/ctubbsii
> > > > >
> > > > >
> > > > > On Thu, Jun 19, 2014 at 11:15 AM, Christopher <ctubbsii@apache.org
> >
> > > > wrote:
> > > > >
> > > > > > Agreed. That's a minimum.
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Christopher L Tubbs II
> > > > > > http://gravatar.com/ctubbsii
> > > > > >
> > > > > >
> > > > > > On Thu, Jun 19, 2014 at 10:54 AM, Bill Havanki <
> > > > > bhavanki@clouderagovt.com>
> > > > > > wrote:
> > > > > >
> > > > > >> I've filed ACCUMULO-2927 to make 'git clean -df' sufficient. No
> > > matter
> > > > > how
> > > > > >> we decide about the rat plugin, I think not requiring -x is a
> > > > worthwhile
> > > > > >> goal.
> > > > > >>
> > > > > >>
> > > > > >> On Tue, Jun 17, 2014 at 5:43 PM, Christopher <
> ctubbsii@apache.org
> > >
> > > > > wrote:
> > > > > >>
> > > > > >> > I agree that instructing users to use this option to modify
> the
> > > > build
> > > > > >> isn't
> > > > > >> > acceptable and I wouldn't recommend this as a response to
> > > users... I
> > > > > was
> > > > > >> > only stating this as a fact, to point out that a special
> profile
> > > on
> > > > by
> > > > > >> > default with an option to disable isn't needed, since that's
> the
> > > > > current
> > > > > >> > behavior.
> > > > > >> >
> > > > > >> > I'm more interested in the targeted .gitignore with the
> > > recommended
> > > > > "git
> > > > > >> > clean -df" option without -x. This helps contributors
> understand
> > > > build
> > > > > >> > tools, makes them aware of the differences between branches,
> and
> > > > > doesn't
> > > > > >> > hide problems introduced by switching branches in an obscure
> > > error,
> > > > > all
> > > > > >> > without blowing away their IDE build files. (though switching
> > > > branches
> > > > > >> > often warrants blowing these IDE files away anyway, since
> > > different
> > > > > >> modules
> > > > > >> > in different branches will be problematic for most IDEs).
> > > > > >> >
> > > > > >> >
> > > > > >> > --
> > > > > >> > Christopher L Tubbs II
> > > > > >> > http://gravatar.com/ctubbsii
> > > > > >> >
> > > > > >> >
> > > > > >> > On Tue, Jun 17, 2014 at 4:47 PM, Alex Moundalexis <
> > > > > >> alexm@clouderagovt.com>
> > > > > >> > 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.
> > > > > >> > >
> > > > > >> > > 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
> > > > > >> > >
> > > > > >> > > On Tue, Jun 17, 2014 at 3:15 PM, Christopher <
> > > ctubbsii@apache.org
> > > > >
> > > > > >> > wrote:
> > > > > >> > >
> > > > > >> > > > There's already a way to skip it for those who don't
> > > understand
> > > > > why
> > > > > >> its
> > > > > >> > > > failing and are incapable/unwilling to troubleshoot:
> > > > > >> > > > -Drat.ignoreErrors=true
> > > > > >> > > >
> > > > > >> > > >
> > > > > >> > > > --
> > > > > >> > > > Christopher L Tubbs II
> > > > > >> > > > http://gravatar.com/ctubbsii
> > > > > >> > > >
> > > > > >> > > >
> > > > > >> > > > On Tue, Jun 17, 2014 at 3:09 PM, Billie Rinaldi <
> > > > > >> > > billie.rinaldi@gmail.com>
> > > > > >> > > > wrote:
> > > > > >> > > >
> > > > > >> > > > > I'm not thrilled about turning it off by default.  How
> > about
> > > > > >> putting
> > > > > >> > it
> > > > > >> > > > in
> > > > > >> > > > > a profile that would be enabled by default, but could be
> > > > > disabled
> > > > > >> > with
> > > > > >> > > a
> > > > > >> > > > > flag for those who don't understand why it's failing?
> > > > > >> > > > >
> > > > > >> > > > >
> > > > > >> > > > > On Tue, Jun 17, 2014 at 11:44 AM, Sean Busbey <
> > > > > >> busbey@cloudera.com>
> > > > > >> > > > wrote:
> > > > > >> > > > >
> > > > > >> > > > > > I've had a few different new-to-Accumulo contributors
> > > > recently
> > > > > >> run
> > > > > >> > > into
> > > > > >> > > > > the
> > > > > >> > > > > > issue of Rat failing the build after changing
> branches.
> > > > > >> > > > > >
> > > > > >> > > > > > I know we already have a warning about this[1], but
> > AFAICT
> > > > > it's
> > > > > >> > over
> > > > > >> > > > the
> > > > > >> > > > > > threshold for consumable information.
> > > > > >> > > > > >
> > > > > >> > > > > > Even after pointing people to the warning, the
> existing
> > > > > >> workaround
> > > > > >> > > > > tripped
> > > > > >> > > > > > up atleast one of them. Despite the warning about
> using
> > > "git
> > > > > >> > clean,"
> > > > > >> > > > the
> > > > > >> > > > > > destruction of their local IDE changes were
> surprising.
> > > > > >> > > > > >
> > > > > >> > > > > > For contributions to Accumulo that aren't coming from
> > > > > >> committers,
> > > > > >> > the
> > > > > >> > > > Rat
> > > > > >> > > > > > plugin seems much more likely to give a false positive
> > > than
> > > > to
> > > > > >> > catch
> > > > > >> > > an
> > > > > >> > > > > > error. Additionally, whatever committer is reviewing
> the
> > > > > >> > contribution
> > > > > >> > > > > > should be checking for license compliance anyways.
> > > > > >> > > > > >
> > > > > >> > > > > > In the interests of reducing the surprise for new
> > > > > contributors,
> > > > > >> I'd
> > > > > >> > > > like
> > > > > >> > > > > to
> > > > > >> > > > > > move our use of Rat to a profile that is only default
> > > > enabled
> > > > > >> > during
> > > > > >> > > a
> > > > > >> > > > > > release run.
> > > > > >> > > > > >
> > > > > >> > > > > > The profile would still let those who want rat to run
> on
> > > > every
> > > > > >> > build
> > > > > >> > > to
> > > > > >> > > > > > enable it and we could update the guide for handling
> new
> > > > > >> > > contributions
> > > > > >> > > > to
> > > > > >> > > > > > say committers should enable the rat profile to help
> > guard
> > > > > >> against
> > > > > >> > > > > errors.
> > > > > >> > > > > >
> > > > > >> > > > > > Any objections?
> > > > > >> > > > > >
> > > > > >> > > > > > [1]:
> > > http://accumulo.apache.org/source.html#running-a-build
> > > > > >> > > > > >
> > > > > >> > > > > > --
> > > > > >> > > > > > Sean
> > > > > >> > > > > >
> > > > > >> > > > >
> > > > > >> > > >
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > > >>
> > > > > >>
> > > > > >> --
> > > > > >> // Bill Havanki
> > > > > >> // Solutions Architect, Cloudera Govt Solutions
> > > > > >> // 443.686.9283
> > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > Sean
> > >
> >
>
>
>
> --
> Sean
>

Re: moving rat to a profile?

Posted by Sean Busbey <bu...@cloudera.com>.
All due respect Christopher, you're a developer with many years of
experience who's worked with this code base for a decent amount of time;
that you don't find the error message problematic has very little bearing
on the fact that new contributors have (and have said as much multiple
times).

I'm not saying that new contributors shouldn't think about licensing at
all. I'm saying that we should ease them into it, similar to how we ease
them into everything else (e.g. integration tests).

I don't believe we are at a point of irreconcilable differences.
Specifically, I haven't heard any technical justification that we are at an
increased risk of improperly licensed files getting into the code base
provided that committers follow the proposed changed instructions to check
contributions for rat failures (esp if they follow the instructions to
default all of their builds to fail on rat errors). Do you believe that our
committers will not follow these instructions? If so, would a git hook that
did the enforcement suffice?

-Sean



On Fri, Jul 11, 2014 at 7:24 AM, Christopher <ct...@apache.org> wrote:

> I'd like to point out that there's a few upstream JIRAs related to this
> conversation that, if resolved, might address all concerns in this thread:
>
> https://issues.apache.org/jira/browse/RAT-61
> https://issues.apache.org/jira/browse/RAT-157
> https://issues.apache.org/jira/browse/RAT-164
>
> To the points made, I really don't find the error message to be that
> problematic for new contributors, especially now that `git status`
> prominently shows the ignored files left behind from a build in a different
> branch. It tells you precisely what the problem is, and which file to look
> in to to see the details. The bare minimum thing a contributor can do is
> read the error message, after all. And, doing so will begin getting them
> into the mindset of understanding the licensing, which is really a
> prerequisite for contributing.
>
> I empathize with the concerns enumerated about the error being related to
> something other than what the user did... but the fact is, it's not
> unrelated to what the user did. It's related to the fact that they are
> using the SCM, building, then switching branches, then building again
> without cleaning up the previous build. This would be equivalent to
> unpacking a tarball and running `make`, then unpacking a different tarball
> over top of the first and running `make install`. It's not unrelated to the
> contributor's actions. However, it is true that they may not understand the
> quirks of maven, git, and the rat plugin, to understand how it is related
> to what they did, and with that I share the expressed frustration, but am
> also more than willing to help any contributors fill this gap in their
> understanding (as I'm sure we all are). I further doubt that this has
> actually caused any possible contributor to be turned away, without so much
> as a question in IRC or on the mailing lists.
>
> At this point, I think there's probably an irreconcilable difference in
> views regarding the default action of this plugin. So, perhaps it might be
> necessary to simply vote on the issue.
>
>
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>
>
> On Fri, Jul 11, 2014 at 7:07 AM, Sean Busbey <bu...@cloudera.com> wrote:
>
> > Changes to ease ignoring the rat report (but leaving it enabled to fail
> the
> > build by default) only help established contributors. Our changes to make
> > clean up of trash easier similarly only help more established
> contributors.
> >
> > We have a duty to help grow the community. A part of that is committers
> > helping to ease new contributors into the requirements that they need to
> > fulfill. We already know that having the rat plugin fail the build, in
> > combination with it having no way to ignore trash from other branches,
> is a
> > stumbling block for new contributors.
> >
> > It is much better to have a committer guide a new contributor through
> > something like a missing license header than it is to have that new
> > contributor run into the current failure.
> >
> > * It's a failed build when they "haven't done anything yet" - something I
> > use as a major red flag about new communities and I'm sure others do as
> > well
> > * It's a failure message that gives you 0 information until you dig into
> > the report
> > * In the most common case, that report tells you it's a problem unrelated
> > to anything you did
> > * It's a problem that isn't fixed with "mvn clean", so new users have to
> > ask for guidance (presuming they don't give up)
> >
> > Provided that we have instructions on how committers should be checking
> new
> > contributions with the plugin combined with instructions for committers
> to
> > default the plugin to fail the build, what is the added risk?
> >
> > We'll have Jenkins as a fall back to keep us honest, just like we do to
> > make sure committers are checking that things compile at all.
> >
> >
> > On Thu, Jul 10, 2014 at 10:25 PM, Eric Newton <er...@gmail.com>
> > wrote:
> >
> > > -1 for removing the rat plugin for default builds
> > > +1 for a simple way to ignore it
> > >
> > > I find Christopher's reasoning sound.  I was guilty of committing half
> > the
> > > non-compliant files before the rat check was on by default.
> > >
> > > I wish the rat check was less annoying about build trash.
> > > I wish the rat check produced error messages that didn't require me to
> > > consult a report.
> > > I wish the template mechanism in eclipse worked every time so source
> > files
> > > would always get the copyright header by default.
> > >
> > > But, automating the check, and moving it to an earlier point in the dev
> > > cycle is a good thing.
> > >
> > > -Eric
> > >
> > >
> > >
> > >
> > >
> > > On Thu, Jul 10, 2014 at 6:09 PM, Christopher <ct...@apache.org>
> > wrote:
> > >
> > > > Since this came up in Review (https://reviews.apache.org/r/23391/),
> > I'm
> > > > revisiting this thread.
> > > >
> > > > In short, it seems there was some confusion on whether or not we had
> > > > reached consensus on enabling rat by default, with
> > rat.ignoreErrors=true
> > > > set to default, and requiring any entities that should be responsible
> > for
> > > > checking legal status of contributions to turn it in (set
> > > > rat.ignoreErrors=false for Jenkins, committers, etc.)
> > > >
> > > > I think one problem is that there was not actually consensus. This
> was
> > a
> > > > proposal made previously by Bill Havanki to do the above, after which
> > we
> > > > agreed to minimally making changes to make "git clean -df"
> sufficient.
> > > But
> > > > there did not appear to be any consensus reached on disabling error
> > > > checking by default, as proposed. Billie had expressed concerns about
> > > > encouraging committers to not take care in checking the legal status
> of
> > > > their commits, and not only do I share those concerns, I'd also
> extend
> > my
> > > > concerns to any contributor. This is simply something that everybody
> > > > contributing to ASF should be aware of. Committers have the extra
> > > > responsibility, but even contributors and potential committers should
> > > > understand the contribution requirements, the development process,
> and
> > > have
> > > > put in some minimal amount of thought about whether they have the
> > ability
> > > > to contribute a particular piece of code legally.
> > > >
> > > > Additionally, I'm concerned about who we'd be easing things for.
> > There's
> > > > clearly an increased risk of commits/contributions without strict
> > > checking
> > > > for licenses (we've seen past commits with these errors... just do
> `git
> > > log
> > > > | grep -i license` to see that). So, we're accepting that risk by
> > making
> > > > the easy route one that serves who, exactly? The ASF makes it quite
> > clear
> > > > that SCM is for developers, not end users, and this is an
> SCM-specific
> > > > problem (specifically branch switching in git).
> > > >
> > > > So, I do not think there is consensus. There are certainly
> outstanding
> > > > concerns for disabling the rat check by default. However, I think
> there
> > > is
> > > > room for further discussion and compromise, and I'd love to consider
> > more
> > > > solutions (or discuss the merits of the proposed solutions). For
> > > instance,
> > > > one area of compromise I've conceded (as stated in the review) is
> that
> > I
> > > > think it'd be a good idea to put the rat check in a profile which can
> > be
> > > > optionally disabled, so that it doesn't take the 3 extra seconds to
> > run,
> > > > which could impact some iterative testing (like running one IT at a
> > time,
> > > > or doing automated `git bisect`).
> > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Christopher L Tubbs II
> > > > http://gravatar.com/ctubbsii
> > > >
> > > >
> > > > On Thu, Jun 19, 2014 at 11:15 AM, Christopher <ct...@apache.org>
> > > wrote:
> > > >
> > > > > Agreed. That's a minimum.
> > > > >
> > > > >
> > > > > --
> > > > > Christopher L Tubbs II
> > > > > http://gravatar.com/ctubbsii
> > > > >
> > > > >
> > > > > On Thu, Jun 19, 2014 at 10:54 AM, Bill Havanki <
> > > > bhavanki@clouderagovt.com>
> > > > > wrote:
> > > > >
> > > > >> I've filed ACCUMULO-2927 to make 'git clean -df' sufficient. No
> > matter
> > > > how
> > > > >> we decide about the rat plugin, I think not requiring -x is a
> > > worthwhile
> > > > >> goal.
> > > > >>
> > > > >>
> > > > >> On Tue, Jun 17, 2014 at 5:43 PM, Christopher <ctubbsii@apache.org
> >
> > > > wrote:
> > > > >>
> > > > >> > I agree that instructing users to use this option to modify the
> > > build
> > > > >> isn't
> > > > >> > acceptable and I wouldn't recommend this as a response to
> > users... I
> > > > was
> > > > >> > only stating this as a fact, to point out that a special profile
> > on
> > > by
> > > > >> > default with an option to disable isn't needed, since that's the
> > > > current
> > > > >> > behavior.
> > > > >> >
> > > > >> > I'm more interested in the targeted .gitignore with the
> > recommended
> > > > "git
> > > > >> > clean -df" option without -x. This helps contributors understand
> > > build
> > > > >> > tools, makes them aware of the differences between branches, and
> > > > doesn't
> > > > >> > hide problems introduced by switching branches in an obscure
> > error,
> > > > all
> > > > >> > without blowing away their IDE build files. (though switching
> > > branches
> > > > >> > often warrants blowing these IDE files away anyway, since
> > different
> > > > >> modules
> > > > >> > in different branches will be problematic for most IDEs).
> > > > >> >
> > > > >> >
> > > > >> > --
> > > > >> > Christopher L Tubbs II
> > > > >> > http://gravatar.com/ctubbsii
> > > > >> >
> > > > >> >
> > > > >> > On Tue, Jun 17, 2014 at 4:47 PM, Alex Moundalexis <
> > > > >> alexm@clouderagovt.com>
> > > > >> > 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.
> > > > >> > >
> > > > >> > > 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
> > > > >> > >
> > > > >> > > On Tue, Jun 17, 2014 at 3:15 PM, Christopher <
> > ctubbsii@apache.org
> > > >
> > > > >> > wrote:
> > > > >> > >
> > > > >> > > > There's already a way to skip it for those who don't
> > understand
> > > > why
> > > > >> its
> > > > >> > > > failing and are incapable/unwilling to troubleshoot:
> > > > >> > > > -Drat.ignoreErrors=true
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > --
> > > > >> > > > Christopher L Tubbs II
> > > > >> > > > http://gravatar.com/ctubbsii
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > On Tue, Jun 17, 2014 at 3:09 PM, Billie Rinaldi <
> > > > >> > > billie.rinaldi@gmail.com>
> > > > >> > > > wrote:
> > > > >> > > >
> > > > >> > > > > I'm not thrilled about turning it off by default.  How
> about
> > > > >> putting
> > > > >> > it
> > > > >> > > > in
> > > > >> > > > > a profile that would be enabled by default, but could be
> > > > disabled
> > > > >> > with
> > > > >> > > a
> > > > >> > > > > flag for those who don't understand why it's failing?
> > > > >> > > > >
> > > > >> > > > >
> > > > >> > > > > On Tue, Jun 17, 2014 at 11:44 AM, Sean Busbey <
> > > > >> busbey@cloudera.com>
> > > > >> > > > wrote:
> > > > >> > > > >
> > > > >> > > > > > I've had a few different new-to-Accumulo contributors
> > > recently
> > > > >> run
> > > > >> > > into
> > > > >> > > > > the
> > > > >> > > > > > issue of Rat failing the build after changing branches.
> > > > >> > > > > >
> > > > >> > > > > > I know we already have a warning about this[1], but
> AFAICT
> > > > it's
> > > > >> > over
> > > > >> > > > the
> > > > >> > > > > > threshold for consumable information.
> > > > >> > > > > >
> > > > >> > > > > > Even after pointing people to the warning, the existing
> > > > >> workaround
> > > > >> > > > > tripped
> > > > >> > > > > > up atleast one of them. Despite the warning about using
> > "git
> > > > >> > clean,"
> > > > >> > > > the
> > > > >> > > > > > destruction of their local IDE changes were surprising.
> > > > >> > > > > >
> > > > >> > > > > > For contributions to Accumulo that aren't coming from
> > > > >> committers,
> > > > >> > the
> > > > >> > > > Rat
> > > > >> > > > > > plugin seems much more likely to give a false positive
> > than
> > > to
> > > > >> > catch
> > > > >> > > an
> > > > >> > > > > > error. Additionally, whatever committer is reviewing the
> > > > >> > contribution
> > > > >> > > > > > should be checking for license compliance anyways.
> > > > >> > > > > >
> > > > >> > > > > > In the interests of reducing the surprise for new
> > > > contributors,
> > > > >> I'd
> > > > >> > > > like
> > > > >> > > > > to
> > > > >> > > > > > move our use of Rat to a profile that is only default
> > > enabled
> > > > >> > during
> > > > >> > > a
> > > > >> > > > > > release run.
> > > > >> > > > > >
> > > > >> > > > > > The profile would still let those who want rat to run on
> > > every
> > > > >> > build
> > > > >> > > to
> > > > >> > > > > > enable it and we could update the guide for handling new
> > > > >> > > contributions
> > > > >> > > > to
> > > > >> > > > > > say committers should enable the rat profile to help
> guard
> > > > >> against
> > > > >> > > > > errors.
> > > > >> > > > > >
> > > > >> > > > > > Any objections?
> > > > >> > > > > >
> > > > >> > > > > > [1]:
> > http://accumulo.apache.org/source.html#running-a-build
> > > > >> > > > > >
> > > > >> > > > > > --
> > > > >> > > > > > Sean
> > > > >> > > > > >
> > > > >> > > > >
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > > >>
> > > > >>
> > > > >> --
> > > > >> // Bill Havanki
> > > > >> // Solutions Architect, Cloudera Govt Solutions
> > > > >> // 443.686.9283
> > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
> >
> >
> > --
> > Sean
> >
>



-- 
Sean

Re: moving rat to a profile?

Posted by Christopher <ct...@apache.org>.
I'd like to point out that there's a few upstream JIRAs related to this
conversation that, if resolved, might address all concerns in this thread:

https://issues.apache.org/jira/browse/RAT-61
https://issues.apache.org/jira/browse/RAT-157
https://issues.apache.org/jira/browse/RAT-164

To the points made, I really don't find the error message to be that
problematic for new contributors, especially now that `git status`
prominently shows the ignored files left behind from a build in a different
branch. It tells you precisely what the problem is, and which file to look
in to to see the details. The bare minimum thing a contributor can do is
read the error message, after all. And, doing so will begin getting them
into the mindset of understanding the licensing, which is really a
prerequisite for contributing.

I empathize with the concerns enumerated about the error being related to
something other than what the user did... but the fact is, it's not
unrelated to what the user did. It's related to the fact that they are
using the SCM, building, then switching branches, then building again
without cleaning up the previous build. This would be equivalent to
unpacking a tarball and running `make`, then unpacking a different tarball
over top of the first and running `make install`. It's not unrelated to the
contributor's actions. However, it is true that they may not understand the
quirks of maven, git, and the rat plugin, to understand how it is related
to what they did, and with that I share the expressed frustration, but am
also more than willing to help any contributors fill this gap in their
understanding (as I'm sure we all are). I further doubt that this has
actually caused any possible contributor to be turned away, without so much
as a question in IRC or on the mailing lists.

At this point, I think there's probably an irreconcilable difference in
views regarding the default action of this plugin. So, perhaps it might be
necessary to simply vote on the issue.



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


On Fri, Jul 11, 2014 at 7:07 AM, Sean Busbey <bu...@cloudera.com> wrote:

> Changes to ease ignoring the rat report (but leaving it enabled to fail the
> build by default) only help established contributors. Our changes to make
> clean up of trash easier similarly only help more established contributors.
>
> We have a duty to help grow the community. A part of that is committers
> helping to ease new contributors into the requirements that they need to
> fulfill. We already know that having the rat plugin fail the build, in
> combination with it having no way to ignore trash from other branches, is a
> stumbling block for new contributors.
>
> It is much better to have a committer guide a new contributor through
> something like a missing license header than it is to have that new
> contributor run into the current failure.
>
> * It's a failed build when they "haven't done anything yet" - something I
> use as a major red flag about new communities and I'm sure others do as
> well
> * It's a failure message that gives you 0 information until you dig into
> the report
> * In the most common case, that report tells you it's a problem unrelated
> to anything you did
> * It's a problem that isn't fixed with "mvn clean", so new users have to
> ask for guidance (presuming they don't give up)
>
> Provided that we have instructions on how committers should be checking new
> contributions with the plugin combined with instructions for committers to
> default the plugin to fail the build, what is the added risk?
>
> We'll have Jenkins as a fall back to keep us honest, just like we do to
> make sure committers are checking that things compile at all.
>
>
> On Thu, Jul 10, 2014 at 10:25 PM, Eric Newton <er...@gmail.com>
> wrote:
>
> > -1 for removing the rat plugin for default builds
> > +1 for a simple way to ignore it
> >
> > I find Christopher's reasoning sound.  I was guilty of committing half
> the
> > non-compliant files before the rat check was on by default.
> >
> > I wish the rat check was less annoying about build trash.
> > I wish the rat check produced error messages that didn't require me to
> > consult a report.
> > I wish the template mechanism in eclipse worked every time so source
> files
> > would always get the copyright header by default.
> >
> > But, automating the check, and moving it to an earlier point in the dev
> > cycle is a good thing.
> >
> > -Eric
> >
> >
> >
> >
> >
> > On Thu, Jul 10, 2014 at 6:09 PM, Christopher <ct...@apache.org>
> wrote:
> >
> > > Since this came up in Review (https://reviews.apache.org/r/23391/),
> I'm
> > > revisiting this thread.
> > >
> > > In short, it seems there was some confusion on whether or not we had
> > > reached consensus on enabling rat by default, with
> rat.ignoreErrors=true
> > > set to default, and requiring any entities that should be responsible
> for
> > > checking legal status of contributions to turn it in (set
> > > rat.ignoreErrors=false for Jenkins, committers, etc.)
> > >
> > > I think one problem is that there was not actually consensus. This was
> a
> > > proposal made previously by Bill Havanki to do the above, after which
> we
> > > agreed to minimally making changes to make "git clean -df" sufficient.
> > But
> > > there did not appear to be any consensus reached on disabling error
> > > checking by default, as proposed. Billie had expressed concerns about
> > > encouraging committers to not take care in checking the legal status of
> > > their commits, and not only do I share those concerns, I'd also extend
> my
> > > concerns to any contributor. This is simply something that everybody
> > > contributing to ASF should be aware of. Committers have the extra
> > > responsibility, but even contributors and potential committers should
> > > understand the contribution requirements, the development process, and
> > have
> > > put in some minimal amount of thought about whether they have the
> ability
> > > to contribute a particular piece of code legally.
> > >
> > > Additionally, I'm concerned about who we'd be easing things for.
> There's
> > > clearly an increased risk of commits/contributions without strict
> > checking
> > > for licenses (we've seen past commits with these errors... just do `git
> > log
> > > | grep -i license` to see that). So, we're accepting that risk by
> making
> > > the easy route one that serves who, exactly? The ASF makes it quite
> clear
> > > that SCM is for developers, not end users, and this is an SCM-specific
> > > problem (specifically branch switching in git).
> > >
> > > So, I do not think there is consensus. There are certainly outstanding
> > > concerns for disabling the rat check by default. However, I think there
> > is
> > > room for further discussion and compromise, and I'd love to consider
> more
> > > solutions (or discuss the merits of the proposed solutions). For
> > instance,
> > > one area of compromise I've conceded (as stated in the review) is that
> I
> > > think it'd be a good idea to put the rat check in a profile which can
> be
> > > optionally disabled, so that it doesn't take the 3 extra seconds to
> run,
> > > which could impact some iterative testing (like running one IT at a
> time,
> > > or doing automated `git bisect`).
> > >
> > >
> > >
> > >
> > > --
> > > Christopher L Tubbs II
> > > http://gravatar.com/ctubbsii
> > >
> > >
> > > On Thu, Jun 19, 2014 at 11:15 AM, Christopher <ct...@apache.org>
> > wrote:
> > >
> > > > Agreed. That's a minimum.
> > > >
> > > >
> > > > --
> > > > Christopher L Tubbs II
> > > > http://gravatar.com/ctubbsii
> > > >
> > > >
> > > > On Thu, Jun 19, 2014 at 10:54 AM, Bill Havanki <
> > > bhavanki@clouderagovt.com>
> > > > wrote:
> > > >
> > > >> I've filed ACCUMULO-2927 to make 'git clean -df' sufficient. No
> matter
> > > how
> > > >> we decide about the rat plugin, I think not requiring -x is a
> > worthwhile
> > > >> goal.
> > > >>
> > > >>
> > > >> On Tue, Jun 17, 2014 at 5:43 PM, Christopher <ct...@apache.org>
> > > wrote:
> > > >>
> > > >> > I agree that instructing users to use this option to modify the
> > build
> > > >> isn't
> > > >> > acceptable and I wouldn't recommend this as a response to
> users... I
> > > was
> > > >> > only stating this as a fact, to point out that a special profile
> on
> > by
> > > >> > default with an option to disable isn't needed, since that's the
> > > current
> > > >> > behavior.
> > > >> >
> > > >> > I'm more interested in the targeted .gitignore with the
> recommended
> > > "git
> > > >> > clean -df" option without -x. This helps contributors understand
> > build
> > > >> > tools, makes them aware of the differences between branches, and
> > > doesn't
> > > >> > hide problems introduced by switching branches in an obscure
> error,
> > > all
> > > >> > without blowing away their IDE build files. (though switching
> > branches
> > > >> > often warrants blowing these IDE files away anyway, since
> different
> > > >> modules
> > > >> > in different branches will be problematic for most IDEs).
> > > >> >
> > > >> >
> > > >> > --
> > > >> > Christopher L Tubbs II
> > > >> > http://gravatar.com/ctubbsii
> > > >> >
> > > >> >
> > > >> > On Tue, Jun 17, 2014 at 4:47 PM, Alex Moundalexis <
> > > >> alexm@clouderagovt.com>
> > > >> > 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.
> > > >> > >
> > > >> > > 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
> > > >> > >
> > > >> > > On Tue, Jun 17, 2014 at 3:15 PM, Christopher <
> ctubbsii@apache.org
> > >
> > > >> > wrote:
> > > >> > >
> > > >> > > > There's already a way to skip it for those who don't
> understand
> > > why
> > > >> its
> > > >> > > > failing and are incapable/unwilling to troubleshoot:
> > > >> > > > -Drat.ignoreErrors=true
> > > >> > > >
> > > >> > > >
> > > >> > > > --
> > > >> > > > Christopher L Tubbs II
> > > >> > > > http://gravatar.com/ctubbsii
> > > >> > > >
> > > >> > > >
> > > >> > > > On Tue, Jun 17, 2014 at 3:09 PM, Billie Rinaldi <
> > > >> > > billie.rinaldi@gmail.com>
> > > >> > > > wrote:
> > > >> > > >
> > > >> > > > > I'm not thrilled about turning it off by default.  How about
> > > >> putting
> > > >> > it
> > > >> > > > in
> > > >> > > > > a profile that would be enabled by default, but could be
> > > disabled
> > > >> > with
> > > >> > > a
> > > >> > > > > flag for those who don't understand why it's failing?
> > > >> > > > >
> > > >> > > > >
> > > >> > > > > On Tue, Jun 17, 2014 at 11:44 AM, Sean Busbey <
> > > >> busbey@cloudera.com>
> > > >> > > > wrote:
> > > >> > > > >
> > > >> > > > > > I've had a few different new-to-Accumulo contributors
> > recently
> > > >> run
> > > >> > > into
> > > >> > > > > the
> > > >> > > > > > issue of Rat failing the build after changing branches.
> > > >> > > > > >
> > > >> > > > > > I know we already have a warning about this[1], but AFAICT
> > > it's
> > > >> > over
> > > >> > > > the
> > > >> > > > > > threshold for consumable information.
> > > >> > > > > >
> > > >> > > > > > Even after pointing people to the warning, the existing
> > > >> workaround
> > > >> > > > > tripped
> > > >> > > > > > up atleast one of them. Despite the warning about using
> "git
> > > >> > clean,"
> > > >> > > > the
> > > >> > > > > > destruction of their local IDE changes were surprising.
> > > >> > > > > >
> > > >> > > > > > For contributions to Accumulo that aren't coming from
> > > >> committers,
> > > >> > the
> > > >> > > > Rat
> > > >> > > > > > plugin seems much more likely to give a false positive
> than
> > to
> > > >> > catch
> > > >> > > an
> > > >> > > > > > error. Additionally, whatever committer is reviewing the
> > > >> > contribution
> > > >> > > > > > should be checking for license compliance anyways.
> > > >> > > > > >
> > > >> > > > > > In the interests of reducing the surprise for new
> > > contributors,
> > > >> I'd
> > > >> > > > like
> > > >> > > > > to
> > > >> > > > > > move our use of Rat to a profile that is only default
> > enabled
> > > >> > during
> > > >> > > a
> > > >> > > > > > release run.
> > > >> > > > > >
> > > >> > > > > > The profile would still let those who want rat to run on
> > every
> > > >> > build
> > > >> > > to
> > > >> > > > > > enable it and we could update the guide for handling new
> > > >> > > contributions
> > > >> > > > to
> > > >> > > > > > say committers should enable the rat profile to help guard
> > > >> against
> > > >> > > > > errors.
> > > >> > > > > >
> > > >> > > > > > Any objections?
> > > >> > > > > >
> > > >> > > > > > [1]:
> http://accumulo.apache.org/source.html#running-a-build
> > > >> > > > > >
> > > >> > > > > > --
> > > >> > > > > > Sean
> > > >> > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > > >>
> > > >>
> > > >> --
> > > >> // Bill Havanki
> > > >> // Solutions Architect, Cloudera Govt Solutions
> > > >> // 443.686.9283
> > > >>
> > > >
> > > >
> > >
> >
>
>
>
> --
> Sean
>

Re: moving rat to a profile?

Posted by Sean Busbey <bu...@cloudera.com>.
Changes to ease ignoring the rat report (but leaving it enabled to fail the
build by default) only help established contributors. Our changes to make
clean up of trash easier similarly only help more established contributors.

We have a duty to help grow the community. A part of that is committers
helping to ease new contributors into the requirements that they need to
fulfill. We already know that having the rat plugin fail the build, in
combination with it having no way to ignore trash from other branches, is a
stumbling block for new contributors.

It is much better to have a committer guide a new contributor through
something like a missing license header than it is to have that new
contributor run into the current failure.

* It's a failed build when they "haven't done anything yet" - something I
use as a major red flag about new communities and I'm sure others do as well
* It's a failure message that gives you 0 information until you dig into
the report
* In the most common case, that report tells you it's a problem unrelated
to anything you did
* It's a problem that isn't fixed with "mvn clean", so new users have to
ask for guidance (presuming they don't give up)

Provided that we have instructions on how committers should be checking new
contributions with the plugin combined with instructions for committers to
default the plugin to fail the build, what is the added risk?

We'll have Jenkins as a fall back to keep us honest, just like we do to
make sure committers are checking that things compile at all.


On Thu, Jul 10, 2014 at 10:25 PM, Eric Newton <er...@gmail.com> wrote:

> -1 for removing the rat plugin for default builds
> +1 for a simple way to ignore it
>
> I find Christopher's reasoning sound.  I was guilty of committing half the
> non-compliant files before the rat check was on by default.
>
> I wish the rat check was less annoying about build trash.
> I wish the rat check produced error messages that didn't require me to
> consult a report.
> I wish the template mechanism in eclipse worked every time so source files
> would always get the copyright header by default.
>
> But, automating the check, and moving it to an earlier point in the dev
> cycle is a good thing.
>
> -Eric
>
>
>
>
>
> On Thu, Jul 10, 2014 at 6:09 PM, Christopher <ct...@apache.org> wrote:
>
> > Since this came up in Review (https://reviews.apache.org/r/23391/), I'm
> > revisiting this thread.
> >
> > In short, it seems there was some confusion on whether or not we had
> > reached consensus on enabling rat by default, with rat.ignoreErrors=true
> > set to default, and requiring any entities that should be responsible for
> > checking legal status of contributions to turn it in (set
> > rat.ignoreErrors=false for Jenkins, committers, etc.)
> >
> > I think one problem is that there was not actually consensus. This was a
> > proposal made previously by Bill Havanki to do the above, after which we
> > agreed to minimally making changes to make "git clean -df" sufficient.
> But
> > there did not appear to be any consensus reached on disabling error
> > checking by default, as proposed. Billie had expressed concerns about
> > encouraging committers to not take care in checking the legal status of
> > their commits, and not only do I share those concerns, I'd also extend my
> > concerns to any contributor. This is simply something that everybody
> > contributing to ASF should be aware of. Committers have the extra
> > responsibility, but even contributors and potential committers should
> > understand the contribution requirements, the development process, and
> have
> > put in some minimal amount of thought about whether they have the ability
> > to contribute a particular piece of code legally.
> >
> > Additionally, I'm concerned about who we'd be easing things for. There's
> > clearly an increased risk of commits/contributions without strict
> checking
> > for licenses (we've seen past commits with these errors... just do `git
> log
> > | grep -i license` to see that). So, we're accepting that risk by making
> > the easy route one that serves who, exactly? The ASF makes it quite clear
> > that SCM is for developers, not end users, and this is an SCM-specific
> > problem (specifically branch switching in git).
> >
> > So, I do not think there is consensus. There are certainly outstanding
> > concerns for disabling the rat check by default. However, I think there
> is
> > room for further discussion and compromise, and I'd love to consider more
> > solutions (or discuss the merits of the proposed solutions). For
> instance,
> > one area of compromise I've conceded (as stated in the review) is that I
> > think it'd be a good idea to put the rat check in a profile which can be
> > optionally disabled, so that it doesn't take the 3 extra seconds to run,
> > which could impact some iterative testing (like running one IT at a time,
> > or doing automated `git bisect`).
> >
> >
> >
> >
> > --
> > Christopher L Tubbs II
> > http://gravatar.com/ctubbsii
> >
> >
> > On Thu, Jun 19, 2014 at 11:15 AM, Christopher <ct...@apache.org>
> wrote:
> >
> > > Agreed. That's a minimum.
> > >
> > >
> > > --
> > > Christopher L Tubbs II
> > > http://gravatar.com/ctubbsii
> > >
> > >
> > > On Thu, Jun 19, 2014 at 10:54 AM, Bill Havanki <
> > bhavanki@clouderagovt.com>
> > > wrote:
> > >
> > >> I've filed ACCUMULO-2927 to make 'git clean -df' sufficient. No matter
> > how
> > >> we decide about the rat plugin, I think not requiring -x is a
> worthwhile
> > >> goal.
> > >>
> > >>
> > >> On Tue, Jun 17, 2014 at 5:43 PM, Christopher <ct...@apache.org>
> > wrote:
> > >>
> > >> > I agree that instructing users to use this option to modify the
> build
> > >> isn't
> > >> > acceptable and I wouldn't recommend this as a response to users... I
> > was
> > >> > only stating this as a fact, to point out that a special profile on
> by
> > >> > default with an option to disable isn't needed, since that's the
> > current
> > >> > behavior.
> > >> >
> > >> > I'm more interested in the targeted .gitignore with the recommended
> > "git
> > >> > clean -df" option without -x. This helps contributors understand
> build
> > >> > tools, makes them aware of the differences between branches, and
> > doesn't
> > >> > hide problems introduced by switching branches in an obscure error,
> > all
> > >> > without blowing away their IDE build files. (though switching
> branches
> > >> > often warrants blowing these IDE files away anyway, since different
> > >> modules
> > >> > in different branches will be problematic for most IDEs).
> > >> >
> > >> >
> > >> > --
> > >> > Christopher L Tubbs II
> > >> > http://gravatar.com/ctubbsii
> > >> >
> > >> >
> > >> > On Tue, Jun 17, 2014 at 4:47 PM, Alex Moundalexis <
> > >> alexm@clouderagovt.com>
> > >> > 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.
> > >> > >
> > >> > > 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
> > >> > >
> > >> > > On Tue, Jun 17, 2014 at 3:15 PM, Christopher <ctubbsii@apache.org
> >
> > >> > wrote:
> > >> > >
> > >> > > > There's already a way to skip it for those who don't understand
> > why
> > >> its
> > >> > > > failing and are incapable/unwilling to troubleshoot:
> > >> > > > -Drat.ignoreErrors=true
> > >> > > >
> > >> > > >
> > >> > > > --
> > >> > > > Christopher L Tubbs II
> > >> > > > http://gravatar.com/ctubbsii
> > >> > > >
> > >> > > >
> > >> > > > On Tue, Jun 17, 2014 at 3:09 PM, Billie Rinaldi <
> > >> > > billie.rinaldi@gmail.com>
> > >> > > > wrote:
> > >> > > >
> > >> > > > > I'm not thrilled about turning it off by default.  How about
> > >> putting
> > >> > it
> > >> > > > in
> > >> > > > > a profile that would be enabled by default, but could be
> > disabled
> > >> > with
> > >> > > a
> > >> > > > > flag for those who don't understand why it's failing?
> > >> > > > >
> > >> > > > >
> > >> > > > > On Tue, Jun 17, 2014 at 11:44 AM, Sean Busbey <
> > >> busbey@cloudera.com>
> > >> > > > wrote:
> > >> > > > >
> > >> > > > > > I've had a few different new-to-Accumulo contributors
> recently
> > >> run
> > >> > > into
> > >> > > > > the
> > >> > > > > > issue of Rat failing the build after changing branches.
> > >> > > > > >
> > >> > > > > > I know we already have a warning about this[1], but AFAICT
> > it's
> > >> > over
> > >> > > > the
> > >> > > > > > threshold for consumable information.
> > >> > > > > >
> > >> > > > > > Even after pointing people to the warning, the existing
> > >> workaround
> > >> > > > > tripped
> > >> > > > > > up atleast one of them. Despite the warning about using "git
> > >> > clean,"
> > >> > > > the
> > >> > > > > > destruction of their local IDE changes were surprising.
> > >> > > > > >
> > >> > > > > > For contributions to Accumulo that aren't coming from
> > >> committers,
> > >> > the
> > >> > > > Rat
> > >> > > > > > plugin seems much more likely to give a false positive than
> to
> > >> > catch
> > >> > > an
> > >> > > > > > error. Additionally, whatever committer is reviewing the
> > >> > contribution
> > >> > > > > > should be checking for license compliance anyways.
> > >> > > > > >
> > >> > > > > > In the interests of reducing the surprise for new
> > contributors,
> > >> I'd
> > >> > > > like
> > >> > > > > to
> > >> > > > > > move our use of Rat to a profile that is only default
> enabled
> > >> > during
> > >> > > a
> > >> > > > > > release run.
> > >> > > > > >
> > >> > > > > > The profile would still let those who want rat to run on
> every
> > >> > build
> > >> > > to
> > >> > > > > > enable it and we could update the guide for handling new
> > >> > > contributions
> > >> > > > to
> > >> > > > > > say committers should enable the rat profile to help guard
> > >> against
> > >> > > > > errors.
> > >> > > > > >
> > >> > > > > > Any objections?
> > >> > > > > >
> > >> > > > > > [1]: http://accumulo.apache.org/source.html#running-a-build
> > >> > > > > >
> > >> > > > > > --
> > >> > > > > > Sean
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >>
> > >>
> > >> --
> > >> // Bill Havanki
> > >> // Solutions Architect, Cloudera Govt Solutions
> > >> // 443.686.9283
> > >>
> > >
> > >
> >
>



-- 
Sean

Re: moving rat to a profile?

Posted by Eric Newton <er...@gmail.com>.
-1 for removing the rat plugin for default builds
+1 for a simple way to ignore it

I find Christopher's reasoning sound.  I was guilty of committing half the
non-compliant files before the rat check was on by default.

I wish the rat check was less annoying about build trash.
I wish the rat check produced error messages that didn't require me to
consult a report.
I wish the template mechanism in eclipse worked every time so source files
would always get the copyright header by default.

But, automating the check, and moving it to an earlier point in the dev
cycle is a good thing.

-Eric





On Thu, Jul 10, 2014 at 6:09 PM, Christopher <ct...@apache.org> wrote:

> Since this came up in Review (https://reviews.apache.org/r/23391/), I'm
> revisiting this thread.
>
> In short, it seems there was some confusion on whether or not we had
> reached consensus on enabling rat by default, with rat.ignoreErrors=true
> set to default, and requiring any entities that should be responsible for
> checking legal status of contributions to turn it in (set
> rat.ignoreErrors=false for Jenkins, committers, etc.)
>
> I think one problem is that there was not actually consensus. This was a
> proposal made previously by Bill Havanki to do the above, after which we
> agreed to minimally making changes to make "git clean -df" sufficient. But
> there did not appear to be any consensus reached on disabling error
> checking by default, as proposed. Billie had expressed concerns about
> encouraging committers to not take care in checking the legal status of
> their commits, and not only do I share those concerns, I'd also extend my
> concerns to any contributor. This is simply something that everybody
> contributing to ASF should be aware of. Committers have the extra
> responsibility, but even contributors and potential committers should
> understand the contribution requirements, the development process, and have
> put in some minimal amount of thought about whether they have the ability
> to contribute a particular piece of code legally.
>
> Additionally, I'm concerned about who we'd be easing things for. There's
> clearly an increased risk of commits/contributions without strict checking
> for licenses (we've seen past commits with these errors... just do `git log
> | grep -i license` to see that). So, we're accepting that risk by making
> the easy route one that serves who, exactly? The ASF makes it quite clear
> that SCM is for developers, not end users, and this is an SCM-specific
> problem (specifically branch switching in git).
>
> So, I do not think there is consensus. There are certainly outstanding
> concerns for disabling the rat check by default. However, I think there is
> room for further discussion and compromise, and I'd love to consider more
> solutions (or discuss the merits of the proposed solutions). For instance,
> one area of compromise I've conceded (as stated in the review) is that I
> think it'd be a good idea to put the rat check in a profile which can be
> optionally disabled, so that it doesn't take the 3 extra seconds to run,
> which could impact some iterative testing (like running one IT at a time,
> or doing automated `git bisect`).
>
>
>
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>
>
> On Thu, Jun 19, 2014 at 11:15 AM, Christopher <ct...@apache.org> wrote:
>
> > Agreed. That's a minimum.
> >
> >
> > --
> > Christopher L Tubbs II
> > http://gravatar.com/ctubbsii
> >
> >
> > On Thu, Jun 19, 2014 at 10:54 AM, Bill Havanki <
> bhavanki@clouderagovt.com>
> > wrote:
> >
> >> I've filed ACCUMULO-2927 to make 'git clean -df' sufficient. No matter
> how
> >> we decide about the rat plugin, I think not requiring -x is a worthwhile
> >> goal.
> >>
> >>
> >> On Tue, Jun 17, 2014 at 5:43 PM, Christopher <ct...@apache.org>
> wrote:
> >>
> >> > I agree that instructing users to use this option to modify the build
> >> isn't
> >> > acceptable and I wouldn't recommend this as a response to users... I
> was
> >> > only stating this as a fact, to point out that a special profile on by
> >> > default with an option to disable isn't needed, since that's the
> current
> >> > behavior.
> >> >
> >> > I'm more interested in the targeted .gitignore with the recommended
> "git
> >> > clean -df" option without -x. This helps contributors understand build
> >> > tools, makes them aware of the differences between branches, and
> doesn't
> >> > hide problems introduced by switching branches in an obscure error,
> all
> >> > without blowing away their IDE build files. (though switching branches
> >> > often warrants blowing these IDE files away anyway, since different
> >> modules
> >> > in different branches will be problematic for most IDEs).
> >> >
> >> >
> >> > --
> >> > Christopher L Tubbs II
> >> > http://gravatar.com/ctubbsii
> >> >
> >> >
> >> > On Tue, Jun 17, 2014 at 4:47 PM, Alex Moundalexis <
> >> alexm@clouderagovt.com>
> >> > 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.
> >> > >
> >> > > 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
> >> > >
> >> > > On Tue, Jun 17, 2014 at 3:15 PM, Christopher <ct...@apache.org>
> >> > wrote:
> >> > >
> >> > > > There's already a way to skip it for those who don't understand
> why
> >> its
> >> > > > failing and are incapable/unwilling to troubleshoot:
> >> > > > -Drat.ignoreErrors=true
> >> > > >
> >> > > >
> >> > > > --
> >> > > > Christopher L Tubbs II
> >> > > > http://gravatar.com/ctubbsii
> >> > > >
> >> > > >
> >> > > > On Tue, Jun 17, 2014 at 3:09 PM, Billie Rinaldi <
> >> > > billie.rinaldi@gmail.com>
> >> > > > wrote:
> >> > > >
> >> > > > > I'm not thrilled about turning it off by default.  How about
> >> putting
> >> > it
> >> > > > in
> >> > > > > a profile that would be enabled by default, but could be
> disabled
> >> > with
> >> > > a
> >> > > > > flag for those who don't understand why it's failing?
> >> > > > >
> >> > > > >
> >> > > > > On Tue, Jun 17, 2014 at 11:44 AM, Sean Busbey <
> >> busbey@cloudera.com>
> >> > > > wrote:
> >> > > > >
> >> > > > > > I've had a few different new-to-Accumulo contributors recently
> >> run
> >> > > into
> >> > > > > the
> >> > > > > > issue of Rat failing the build after changing branches.
> >> > > > > >
> >> > > > > > I know we already have a warning about this[1], but AFAICT
> it's
> >> > over
> >> > > > the
> >> > > > > > threshold for consumable information.
> >> > > > > >
> >> > > > > > Even after pointing people to the warning, the existing
> >> workaround
> >> > > > > tripped
> >> > > > > > up atleast one of them. Despite the warning about using "git
> >> > clean,"
> >> > > > the
> >> > > > > > destruction of their local IDE changes were surprising.
> >> > > > > >
> >> > > > > > For contributions to Accumulo that aren't coming from
> >> committers,
> >> > the
> >> > > > Rat
> >> > > > > > plugin seems much more likely to give a false positive than to
> >> > catch
> >> > > an
> >> > > > > > error. Additionally, whatever committer is reviewing the
> >> > contribution
> >> > > > > > should be checking for license compliance anyways.
> >> > > > > >
> >> > > > > > In the interests of reducing the surprise for new
> contributors,
> >> I'd
> >> > > > like
> >> > > > > to
> >> > > > > > move our use of Rat to a profile that is only default enabled
> >> > during
> >> > > a
> >> > > > > > release run.
> >> > > > > >
> >> > > > > > The profile would still let those who want rat to run on every
> >> > build
> >> > > to
> >> > > > > > enable it and we could update the guide for handling new
> >> > > contributions
> >> > > > to
> >> > > > > > say committers should enable the rat profile to help guard
> >> against
> >> > > > > errors.
> >> > > > > >
> >> > > > > > Any objections?
> >> > > > > >
> >> > > > > > [1]: http://accumulo.apache.org/source.html#running-a-build
> >> > > > > >
> >> > > > > > --
> >> > > > > > Sean
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >>
> >>
> >> --
> >> // Bill Havanki
> >> // Solutions Architect, Cloudera Govt Solutions
> >> // 443.686.9283
> >>
> >
> >
>