OGM-317 : Establish profile for QA plug-ins

[ajaybhat]

You can use the following command to skip checkstyle checks
mvn clean install -s settings-example.xml -DskipQA

This can be extended to other plugins like findbugs also.

Please test and review

[Hibernate-CI]

Can one of the admins add this person to the trusted builders? (reply with: "add to whitelist" or "ok to test")

[Sanne]

ok to test

[DavideD]

Instead of creating a new profile couldn't we just set the property of the checkstyle maven plugin failsOnError to false?
This will still run checkstyle but it won't fail the build in case of errors.

The patch you have submitted changes a lot of spaces and indentations in some part of the code that are note releated to the issue you are trying to solve.
In general, we try to avoid this kind of changes becuase the increase the chance of conlficts and they make the review process much harder. Do you think you can change the pull request (or send a new one, if you prefer) that does not change the XML general format?

Thanks

[ajaybhat]

Checkstyle can already be disabled during build with -Dcheckstyle.skip=true in the maven goals so I don't think changing failsOnError property would be helpful.

[ajaybhat]

No, it's not really required to specify the modules in the profile. Should they be removed?

Is it ok to use more commits to revert the formatting changes?

[DavideD]

No, it's not really required to specify the modules in the profile. Should they be removed?

Yes, remove the modules from the profile, otherwise we would have two places to keep in sync when adding or changing modules.

You can use separate commit

[gunnarmorling]

Instead of creating a new profile couldn't we just set the property of the checkstyle maven plugin failsOnError to false?

The idea was more to provide a switch for excluding all "slow" stuff from running at all. It only really makes sense though if other things than CS would be disabled by such switch, as CS alone can already be disabled via "-Dcheckstyle.skip=true".

Personally I'm fine with using the existing options. But @emmanuelbernard seems to like an easy option for just doing a "quick build" resulting in fast feedback, ommitting all "superfluous stuff" such as CS, doc build, dist build etc.

So we need to agree on what that "quick mode" would comprise. To me, it should be equivalent to doing the following (which is what I basically invoke manually atm.):

mvn clean install \
    -pl :hibernate-ogm-core,:hibernate-ogm-infinispan,:hibernate-ogm-ehcache,:hibernate-ogm-mongodb,:hibernate-ogm-neo4j,:hibernate-ogm-couchdb \
    -Dcheckstyle.skip=true \
    -Denforcer.skip=true

[gunnarmorling]

Is it ok to use more commits to revert the formatting changes?

Either that, or, preferrably, re-work the existing commit. Thanks for your help btw. :)

[ajaybhat]

Warning link

The above error occurs because the child poms have checkstyle plugin specified in < plugins >, but no version for those. So I removed those and tested with the skipQA profile, and it works as expected, Pls do a review

[gunnarmorling]

I'd prefer to leave the configuration of the plug-in version in the main pluginManagement block. That way the plug-in e.g. can be skipped for single modules if needed, without the warning about the missing version. I.e. like so:

<build>
    <pluginManagement>
        <plugins>
            <plugin>
                <artifactId>maven-checkstyle-plugin</artifactId>
                <version>2.12.1</version>
            </plugin>
    ...

<profiles>
    <profile>
        <id>qa</id>
        ...
        <build>
            <plugins>
               <plugin>
                   <artifactId>maven-checkstyle-plugin</artifactId>
                   <configuration>...</configuration>
                   <executions>
                ...

[gunnarmorling]

The test build failed as this change applies CS to some more modules than before (specifically, the perf test module) and there are some glitches in there.

Would you mind taking a look at those? If you don't feel like ironing out these left-overs, just let me know. Shouldn't take long, though ;)

Thanks a lot for your help, Ajay!

[gunnarmorling]

Btw. as MNG-4565 has been fixed in Maven 3.2.2 it should be possible now to establish an "exclude all slow stuff" switch. To all the profiles you don't want to run during a quick dev build (integrationtest, docs, dist, qa), such property could be added to the activation properties. Then running

mvn clean install -Ddevtest

should exclude all these additional profiles. Do you feel like giving this a try?

[gunnarmorling]

@ajaybhat Did you ever come to put some more thought into the last proposal above, i.e. to have one switch to exclude all modules / plug-ins which are not needed for a quick test run during development?

[emmanuelbernard]

What should we do about this PR?

[gunnarmorling]

I would close it. In its current form it doesn't provide much benefits over the status quo.