Hi We discussed about that at the beginning of the project. We agreed to execute rat and findbugs in a specific profile to reduce the build time for dev.
That's why I do mvn clean install -Prelease before submitting a PR and just clean install when I'm developing. No problem to change that. Regards JB On Feb 10, 2017, 07:51, at 07:51, Aviem Zur <[email protected]> wrote: >Can we consider adding rat-plugin and findbugs to the default verify >phase? >Currently they only run when the `release` profile is enabled. > >On Thu, Jan 26, 2017 at 11:42 AM Aljoscha Krettek <[email protected]> >wrote: > >> +1 to what Dan said >> >> On Wed, 25 Jan 2017 at 21:40 Kenneth Knowles <[email protected]> >> wrote: >> >> > +1 >> > >> > On Jan 25, 2017 11:15, "Jean-Baptiste Onofré" <[email protected]> >wrote: >> > >> > > +1 >> > > >> > > It sounds good to me. >> > > >> > > Thanks Dan ! >> > > >> > > Regards >> > > JB >> > > >> > > On Jan 25, 2017, 19:39, at 19:39, Dan Halperin >> > <[email protected]> >> > > wrote: >> > > >Here is my summary of the threads: >> > > > >> > > >Overwhelming agreement: >> > > > >> > > >- rename `release` to something more appropriate. >> > > >- add `checkstyle` to the default build (it's basically a >compile >> > > >error) >> > > >- add more information to contributor guide >> > > > >> > > >Reasonable agreement >> > > > >> > > >- don't update the github instructions to make passing `mvn >verify >> > > >-P<all >> > > >checks>` mandatory. Maybe add a hint that this is a good proxy >for >> what >> > > >Jenkins will run. >> > > > >> > > >Unresolved: >> > > > >> > > >- whether all checks should be in `mvn verify` >> > > >- whether `mvn test` is useful for most workflows >> > > > >> > > >I'll propose to proceed with the overwhelmingly agreed-upon >changes, >> > > >and as >> > > >we see increasingly many new contributors re-evaluate the >remaining >> > > >issues. >> > > > >> > > >Thanks, >> > > >Dan >> > > > >> > > >On Tue, Jan 24, 2017 at 12:51 PM, Jean-Baptiste Onofré >> > > ><[email protected]> >> > > >wrote: >> > > > >> > > >> +1 to at least update the contribution guide and improve the >profile >> > > >name. >> > > >> >> > > >> Regards >> > > >> JB >> > > >> >> > > >> >> > > >> On 01/24/2017 09:49 PM, Kenneth Knowles wrote: >> > > >> >> > > >>> My impression is that we don't have consensus on whether all >checks >> > > >or >> > > >>> minimal checks should be the default, or whether we can have >both >> > > >via `mvn >> > > >>> test` and `mvn verify`. >> > > >>> >> > > >>> But that doesn't prevent us from giving -P release a better >name >> and >> > > >>> mentioning it in the dev guide and in some manner in our PR >> > > >template. >> > > >>> >> > > >>> Right now we are living with the combination of the bad >aspects - >> > > >default >> > > >>> is not thorough but not actually fast and a thorough check is >> > > >>> undocumented. >> > > >>> >> > > >>> On Tue, Jan 24, 2017 at 2:22 AM, Ismaël Mejía ><[email protected]> >> > > >wrote: >> > > >>> >> > > >>> I just wanted to know if we have achieved some consensus >about >> this, >> > > >I >> > > >>>> just >> > > >>>> saw this PR that reminded me about this discussion. >> > > >>>> >> > > >>>> https://github.com/apache/beam/pull/1829 >> > > >>>> >> > > >>>> It is important that we mention the existing profiles (and >the >> > > >intended >> > > >>>> checks) in the contribution guide (e.g. -Prelease (or >-Pall-checks >> > > >>>> triggers >> > > >>>> these validations). >> > > >>>> >> > > >>>> I can add this to the guide if you like once we define the >checks >> > > >per >> > > >>>> stage/profile. >> > > >>>> >> > > >>>> Ismaël >> > > >>>> >> > > >>>> >> > > >>>> On Wed, Jan 11, 2017 at 8:12 AM, Aviem Zur ><[email protected]> >> > > >wrote: >> > > >>>> >> > > >>>> I agree with Dan and Lukasz. >> > > >>>>> Developers should not be expected to know beforehand which >> > > >specific >> > > >>>>> profiles to run. >> > > >>>>> The phase specified in the PR instructions (`verify`) >should run >> > > >all the >> > > >>>>> relevant verifications and be the "slower" build, while a >> > > >preceding >> > > >>>>> lifecycle, such as `test`, should run the "faster" >verifications. >> > > >>>>> >> > > >>>>> Aviem. >> > > >>>>> >> > > >>>>> On Mon, Jan 9, 2017 at 7:57 PM Robert Bradshaw >> > > >>>>> >> > > >>>> <[email protected] >> > > >>>> >> > > >>>>> >> > > >>>>>> wrote: >> > > >>>>> >> > > >>>>> On Mon, Jan 9, 2017 at 3:49 AM, Aljoscha Krettek >> > > ><[email protected]> >> > > >>>>>> wrote: >> > > >>>>>> >> > > >>>>>>> I also usually prefer "mvn verify" to to the expected >thing but >> > > >I see >> > > >>>>>>> >> > > >>>>>> that >> > > >>>>>> >> > > >>>>>>> quick iteration times are key. >> > > >>>>>>> >> > > >>>>>> >> > > >>>>>> I see >> > > >>>>>> https://maven.apache.org/guides/introduction/ >> > > >>>>>> >> > > >>>>> introduction-to-the-lifecycle.html >> > > >>>>> >> > > >>>>>> >> > > >>>>>> verify - run any checks on results of integration >tests to >> > > >ensure >> > > >>>>>> quality criteria are met >> > > >>>>>> >> > > >>>>>> Of course our integration tests are long enough that we >> shouldn't >> > > >be >> > > >>>>>> putting all of them here, but I too would expect >checkstyle. >> > > >>>>>> >> > > >>>>>> Perhaps we could introduce a verify-fast or somesuch for >fast >> > > >(but >> > > >>>>>> lower coverage) turnaround time. I would expect "mvn >verify >> test" >> > > >to >> > > >>>>>> pass before submitting a PR, and would want to run that >before >> > > >asking >> > > >>>>>> others to look at it. I think this should be our criteria >(i.e. >> > > >what >> > > >>>>>> will a new but maven-savvy user run before pushing their >code). >> > > >>>>>> >> > > >>>>>> As long as the pre-commit hooks still check everything I'm >ok >> > > >with >> > > >>>>>>> >> > > >>>>>> making >> > > >>>>> >> > > >>>>>> the default a little more lightweight. >> > > >>>>>>> >> > > >>>>>> >> > > >>>>>> The fact that our pre-commit hooks take a long time to run >does >> > > >change >> > > >>>>>> things. Nothing more annoying than seeing that your PR >failed 3 >> > > >hours >> > > >>>>>> later because you had some trailing whitespace... >> > > >>>>>> >> > > >>>>>> On Thu, 5 Jan 2017 at 21:49 Lukasz Cwik >> > > ><[email protected]> >> > > >>>>>>> >> > > >>>>>> wrote: >> > > >>>>>> >> > > >>>>>>> >> > > >>>>>>> I was hoping that the default mvn verify would be the >slow >> build >> > > >>>>>>>> >> > > >>>>>>> and a >> > > >>>> >> > > >>>>> profile could be enabled that would skip checks to make >things >> > > >>>>>>>> >> > > >>>>>>> faster >> > > >>>> >> > > >>>>> for >> > > >>>>>> >> > > >>>>>>> regular contributors. This way a person doesn't need to >have >> > > >>>>>>>> >> > > >>>>>>> detailed >> > > >>>> >> > > >>>>> knowledge of all our profiles and what they do (typically >mvn >> > > >>>>>>>> >> > > >>>>>>> verify) >> > > >>>> >> > > >>>>> will >> > > >>>>>> >> > > >>>>>>> do the right thing most of the time. >> > > >>>>>>>> >> > > >>>>>>>> On Thu, Jan 5, 2017 at 9:30 AM, Dan Halperin >> > > >>>>>>>> >> > > >>>>>>> <[email protected]> >> > > >>>>>> >> > > >>>>>>> wrote: >> > > >>>>>>>> >> > > >>>>>>>> On Thu, Jan 5, 2017 at 9:28 AM, Jesse Anderson < >> > > >>>>>>>>> >> > > >>>>>>>> [email protected] >> > > >>>>> >> > > >>>>>> >> > > >>>>>>> wrote: >> > > >>>>>>>>> >> > > >>>>>>>>> @dan are you saying that mvn verify isn't doing >checkstyle >> > > >>>>>>>>>> >> > > >>>>>>>>> anymore? >> > > >>>>> >> > > >>>>>> >> > > >>>>>>>>> >> > > >>>>>>>>> `mvn verify` alone should not be running checkstyle, if >> > > >modules >> > > >>>>>>>>> >> > > >>>>>>>> are >> > > >>>> >> > > >>>>> configured correctly. >> > > >>>>>>>>> >> > > >>>>>>>>> >> > > >>>>>>>>> Some of >> > > >>>>>>>>>> the checkstyles are still running for a few modules. >Also, >> > > >the >> > > >>>>>>>>>> >> > > >>>>>>>>> contribution >> > > >>>>>>>>> >> > > >>>>>>>>>> docs will need to change. >> > > >>>>>>>>>> >> > > >>>>>>>>> >> > > >>>>>>>>> >> > > >>>>>>>>> Yes. The PR includes discussion of these other needed >> changes, >> > > >>>>>>>>> unfortunately one PR can't change two repositories. >> > > >>>>>>>>> >> > > >>>>>>>>> Please continue the discussion on the PR, then I will >> > > >summarize it >> > > >>>>>>>>> >> > > >>>>>>>> back >> > > >>>>>> >> > > >>>>>>> into the dev thread. >> > > >>>>>>>>> >> > > >>>>>>>>> Thanks, >> > > >>>>>>>>> Dan >> > > >>>>>>>>> >> > > >>>>>>>>> >> > > >>>>>>>>> They say to run mvn verify before commits. >> > > >>>>>>>>>> >> > > >>>>>>>>>> On Thu, Jan 5, 2017 at 9:25 AM Dan Halperin >> > > >>>>>>>>>> >> > > >>>>>>>>> <[email protected] >> > > >>>>>>>> >> > > >>>>>>>>> >> > > >>>>>>>>>> wrote: >> > > >>>>>>>>>> >> > > >>>>>>>>>> Several folks seem to have been confused after >BEAM-246, >> > > >where >> > > >>>>>>>>>>> >> > > >>>>>>>>>> we >> > > >>>>> >> > > >>>>>> moved >> > > >>>>>>>> >> > > >>>>>>>>> the >> > > >>>>>>>>>> >> > > >>>>>>>>>>> "slow things" into the release profile. I've started >a >> > > >>>>>>>>>>> >> > > >>>>>>>>>> discussion >> > > >>>>> >> > > >>>>>> with >> > > >>>>>>>> >> > > >>>>>>>>> https://github.com/apache/beam/pull/1740 to see if >there are >> > > >>>>>>>>>>> >> > > >>>>>>>>>> things >> > > >>>>>> >> > > >>>>>>> we >> > > >>>>>>>> >> > > >>>>>>>>> can >> > > >>>>>>>>>> >> > > >>>>>>>>>>> do to fill these gaps. >> > > >>>>>>>>>>> >> > > >>>>>>>>>>> Would love folks to chime in with opinions. >> > > >>>>>>>>>>> >> > > >>>>>>>>>>> Dan >> > > >>>>>>>>>>> >> > > >>>>>>>>>>> On Wed, Jan 4, 2017 at 1:34 PM, Jesse Anderson < >> > > >>>>>>>>>>> >> > > >>>>>>>>>> [email protected]> >> > > >>>>>>>> >> > > >>>>>>>>> wrote: >> > > >>>>>>>>>>> >> > > >>>>>>>>>>> @Eugene, yes that failed on the checkstyle. >> > > >>>>>>>>>>>> >> > > >>>>>>>>>>>> On Wed, Jan 4, 2017 at 1:27 PM Eugene Kirpichov >> > > >>>>>>>>>>>> <[email protected]> wrote: >> > > >>>>>>>>>>>> >> > > >>>>>>>>>>>> Try just -Prelease. >> > > >>>>>>>>>>>>> On Wed, Jan 4, 2017 at 1:21 PM Jesse Anderson < >> > > >>>>>>>>>>>>> >> > > >>>>>>>>>>>> [email protected] >> > > >>>>>>>>> >> > > >>>>>>>>>> >> > > >>>>>>>>>>> wrote: >> > > >>>>>>>>>>>>> >> > > >>>>>>>>>>>>> Fails because I don't have a secret key. >> > > >>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>> On Wed, Jan 4, 2017 at 1:03 PM Jean-Baptiste >Onofré < >> > > >>>>>>>>>>>>>> >> > > >>>>>>>>>>>>> [email protected] >> > > >>>>>>>>>> >> > > >>>>>>>>>>> >> > > >>>>>>>>>>>> wrote: >> > > >>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>> Hi Jesse, >> > > >>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>> Could you try the same with: >> > > >>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>> mvn verify -Prelease,apache-release >> > > >>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>> ? >> > > >>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>> Regards >> > > >>>>>>>>>>>>>>> JB >> > > >>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>> On 01/04/2017 09:53 PM, Jesse Anderson wrote: >> > > >>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>>> For some reason, running "mvn verify" isn't >running >> > > >>>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>> checkstyle >> > > >>>>>>>>> >> > > >>>>>>>>>> on >> > > >>>>>>>>>> >> > > >>>>>>>>>>> everything. I had checkstyle errors in >> > > >>>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>> beam-sdks-java-core >> > > >>>>>> >> > > >>>>>>> that >> > > >>>>>>>>> >> > > >>>>>>>>>> weren't >> > > >>>>>>>>>>>>> >> > > >>>>>>>>>>>>>> being found. >> > > >>>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>>> I thought this was due to the extra parameters. >I >> > > >>>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>> reran >> > > >>>>> >> > > >>>>>> with >> > > >>>>>>>> >> > > >>>>>>>>> the >> > > >>>>>>>>>> >> > > >>>>>>>>>>> plain >> > > >>>>>>>>>>>>> >> > > >>>>>>>>>>>>>> "mvn >> > > >>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>>> verify" and it still didn't find them. From the >> > > >>>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>> output, >> > > >>>>> >> > > >>>>>> it >> > > >>>>>> >> > > >>>>>>> doesn't >> > > >>>>>>>>>>> >> > > >>>>>>>>>>>> look >> > > >>>>>>>>>>>>> >> > > >>>>>>>>>>>>>> like they're being run at all. >> > > >>>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>>> Thanks, >> > > >>>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>>> Jesse >> > > >>>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>> -- >> > > >>>>>>>>>>>>>>> Jean-Baptiste Onofré >> > > >>>>>>>>>>>>>>> [email protected] >> > > >>>>>>>>>>>>>>> http://blog.nanthrax.net >> > > >>>>>>>>>>>>>>> Talend - http://www.talend.com >> > > >>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>>> >> > > >>>>>>>>>>>>>> >> > > >>>>>>>>>>>>> >> > > >>>>>>>>>>>> >> > > >>>>>>>>>>> >> > > >>>>>>>>>> >> > > >>>>>>>>> >> > > >>>>>>>> >> > > >>>>>> >> > > >>>>> >> > > >>>> >> > > >>> >> > > >> -- >> > > >> Jean-Baptiste Onofré >> > > >> [email protected] >> > > >> http://blog.nanthrax.net >> > > >> Talend - http://www.talend.com >> > > >> >> > > >> > >>
