> On July 10, 2014, 8:42 p.m., Christopher Tubbs wrote: > > -1 > > There's absolutely no reason to create a profile and options to skip the > > execution of the plugin since there is a built-in mechanism for controlling > > the rat check's impact on the build (eg. by setting rat.ignoreErrors). This > > change unnecessarily complicates the pom files, with insufficient utility > > to justify it, because the rat check is relatively fast, and there's > > already a mechanism to ignore its results. Even if we were able to come to > > consensus on defaulting rat.ignoreErrors to true, I'd still be opposed to > > the added profiles and property to control that profile, due to this added > > complexity with almost no utility. > > Sean Busbey wrote: > Even with ignore errors on, the plugin itself running introduces overhead > for those attempting to iterate tightly on e.g. a particular IT or unit test. > > "Relatively fast" is subjective. I know that both myself and Keith have > complained about the overhead in the past. > > John Vines wrote: > Review board is not a place for discussions about whether or not work > should be done. This is a place for analyzing patches, plain and simple. > Please take this conversation to JIRA. > > Christopher Tubbs wrote: > FWIW, I just did 'mvn validate' with and without rat defined in the pom, > and it was a total of 3 seconds difference (~7 seconds, vs. ~4 seconds). > > I'd concede to moving the existing rat plugin moved to a single profile, > so that it could be disabled with "-P '!rat'" (I think that syntax is > correct) or with the presence of a property. That would allow disabling, for > tight iterations of tests, but with minimal added complexity. The other > option would be to patch rat upstream to have a proper skip (rather than only > a process-and-ignore) option.
latest patch moves to a single profile. > On July 10, 2014, 8:42 p.m., Christopher Tubbs wrote: > > pom.xml, line 133 > > <https://reviews.apache.org/r/23391/diff/1/?file=627668#file627668line133> > > > > -1 > > Ensuring licenses get checked when new code is added (not just at > > release time) is too important of a legal requirement to bypass by default. > > In my view, it absolutely should run as often as possible whenever a change > > is made, and errors should disrupt the build, so they will be taken > > seriously. > > > > Even the rat documentation strongly recommends doing this, and I don't > > think a compelling case has been made to override that recommendation. > > Sean Busbey wrote: > The consensus in the discussion thread was > > * Default fail to false (so new contributors don't get caught up in false > positives) > * Instruct committers to run with failure causing build failure (see > ticket for changes to contribution handling instructions) > * Have Jenkins perform regular enforcement check > > Am I missing something from the conversation? If not, could you please > pick up the thread with your objections? latest patch includes a link to where the documentation will be for committers to default to "rat errors fail build" - Sean ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23391/#review47614 ----------------------------------------------------------- On July 10, 2014, 10:10 p.m., Sean Busbey wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23391/ > ----------------------------------------------------------- > > (Updated July 10, 2014, 10:10 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2986 > https://issues.apache.org/jira/browse/ACCUMULO-2986 > > > Repository: accumulo > > > Description > ------- > > * puts rat plugin into profile > * activates profile by default > * ignores rat errrors by default > > > Diffs > ----- > > pom.xml 2bc87cf082dfeb7bfe1a3fac1fe4fba1eaa87edd > > Diff: https://reviews.apache.org/r/23391/diff/ > > > Testing > ------- > > verified rat warnings and failures given move from master -> 1.6.1-SNAPSHOT > branch with no definitions, -Drat.skip=true, -Drat.skip=false, > -Drat.ignoreErrors=true, and profile to check errors in ~/.m2/settings.xml > > > Thanks, > > Sean Busbey > >
