On Mon, Jan 9, 2017 at 3:49 AM, Aljoscha Krettek <aljos...@apache.org> 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 <lc...@google.com.invalid> 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 <dhalp...@google.com.invalid>
>> wrote:
>>
>> > On Thu, Jan 5, 2017 at 9:28 AM, Jesse Anderson <je...@smokinghand.com>
>> > 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
>> <dhalp...@google.com.invalid
>> > >
>> > > 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 <
>> je...@smokinghand.com>
>> > > > wrote:
>> > > >
>> > > > > @Eugene, yes that failed on the checkstyle.
>> > > > >
>> > > > > On Wed, Jan 4, 2017 at 1:27 PM Eugene Kirpichov
>> > > > > <kirpic...@google.com.invalid> wrote:
>> > > > >
>> > > > > > Try just -Prelease.
>> > > > > > On Wed, Jan 4, 2017 at 1:21 PM Jesse Anderson <
>> > je...@smokinghand.com
>> > > >
>> > > > > > wrote:
>> > > > > >
>> > > > > > > Fails because I don't have a secret key.
>> > > > > > >
>> > > > > > > On Wed, Jan 4, 2017 at 1:03 PM Jean-Baptiste Onofré <
>> > > j...@nanthrax.net
>> > > > >
>> > > > > > > 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é
>> > > > > > > > jbono...@apache.org
>> > > > > > > > http://blog.nanthrax.net
>> > > > > > > > Talend - http://www.talend.com
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>

Reply via email to