search

sfeir-open-source / sonar-clover

It provides the ability to feed SonarQube with code coverage data coming from Atlassian Clover
Apache License 2.0
15 stars 25 forks source link

fix(CI): update sonarsource parent version #19

Closed Tony-Proum closed 5 years ago

mlschechter commented 6 years ago

@Tony-Proum ,

I've already made one of these changes, and can update to address the other if you'd like.

Tony-Proum commented 6 years ago

@mlschechter reviewing your change, I wanted to extract the behavior needs from the technical needs: update parent version and fix CI chain is needed now for the project to be build and allows other contributions to be integrated whereas the your pull request https://github.com/Tony-Proum/sonar-clover/pull/15 purpose is to use the new coverage API.

Also, In my opinion each pull request should contains the minimal changes to get things done in order to help reviews and avoid to introduce things that's not really needed. e.g: https://github.com/Tony-Proum/sonar-clover/pull/15 introduces changes to the copyright section of each files that's need to be studied but are not related to the functional behavior of this pull request

I'm really interested by the work you did, so don't worry, It should be integrated soon, but we need to work on separate thread in order to solve each problem separately.

mlschechter commented 6 years ago

@Tony-Proum,

In general, I agree with you. This was something other than a typical case, though.

The reason I made the sweeping changes I did was that the build and integration tests would not work without them; I had to make all of the changes in my pull request to be able to verify that my modifications worked as expected.

For example, the license changes were required by the Maven plugins used for building; it was easier to make them than try to code around the issue in Travis. with respect to those, I am happy to address any content changes you think we need.

Hope this helps clarify, and please let me know if there's anything I can do to assist.

Tony-Proum commented 5 years ago

This as already been fixed