Sonar Quality Gate

[Łukasz Lewczyński]

Hi all,

The Sonar Quality Gate has been enabled for all core services. I had to create new Jenkins jobs (as pipeline) but I left old for now in case if I missed any configuration. I will probably remove them near the end of the sprint.

For now we use default settings for Quality Gate:

• Coverage on New Code >= 80%
  
• New Bugs == 0
  
• New Vulnerabilities == 0
  
• Technical Debt Ratio on New Code <= 5%

The information about Quality Gate status for each service will be displayed on #build slack channel. In the end when you commit new changes please ensure that Quality Gate pass.

Regards,

Lukasz

Łukasz Lewczyński
Software Developer
llewc...@soldevelo.com

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

[Paweł Albecki]

Hi Łukasz,

This is great news. I have a comment, there is one problem that can be present each time we will add new classes with @EqualsAndHashCode annotation. Sonar shows that some number of conditions are not covered by tests. We don't want to write many unit tests for equals and hashCode methods. But maybe we can use some tools like EqualsVerifier so new code can pass Quality Gate and overall coverage would increase as well?

Best regards,

Paweł

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAAdp53wMMq2WP0M8vPwktBKi3UAUdTo47z7LxJ8U%2Bou9mC550g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--

Paweł Albecki
Software Developer
palb...@soldevelo.com

[Mateusz Kwiatkowski]

Hi Łukasz,

is it possible to have some info about why this quality gate build failed in build details?

Regards,

Mateusz

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAAJzpf%3DODU8XH%3DqRvwUQCuxQDpttiBxNv9fQ6pDmF0aJoQpUBQ%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

[Łukasz Lewczyński]

Paweł Albecki: If a developer add some part of codes (even by adding annotations), those codes should be checked if work properly. I understand that methods like setters, getters, equals, hashCode will probably be implemented correctly but still this is a code that should be tested. I think using the library that you present could be a good way to achieve this.

Mateusz Kwiatkowski: Yes it is possible to add this information but because this is a custom code, each time when we will modify sonar conditions list we will have to update each sonar build on CI. Also in slack message there is a link to sonar results. In my opinion for now we should simple use the link from the slack message.

Łukasz Lewczyński
Software Developer
llewc...@soldevelo.com

[Łukasz Lewczyński]

After I disabled cross project check for code duplication I found out that some services still have some code duplication:

OpenLMIS Fulfillment: 1%
OpenLMIS Reference Data Service: 0.7%
OpenLMIS Stock Management Service: 0.2%

I think it would be good to add new condition to quality date:

Duplicated Lines (%) is less than 5%

What do you think about that?

[Paweł Gesek]

I think that's fine, but if I understand you correctly, all of our services are passing this criteria with ease at the moment, since the highest value is 1%? Shouldn't we target a lower gate in that case?

Regards,

Paweł

--

You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAAdp53yOBnxd43v0pOM-p0uQPj9ZTEUnOJS%2BvBzrnGbKVaodWA%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Paweł Gesek
Technical Project Manager
pge...@soldevelo.com / +48 690 020 875

[Łukasz Lewczyński]

Yes all of services should pass this criteria. In the best scenario this condition should be set to 0% because we don't want to have duplication in the given service. Now I think maybe it would be better to set it to 0% instead of 5%.

Łukasz Lewczyński
Software Developer
llewc...@soldevelo.com

[Paweł Albecki]

I discovered that we have two Sonar Quality Gate profiles:

1. SonarQube way with Metrics
• Coverage on New Code
  
• New Bugs
  
• New Vulnerabilities
  

• Technical Debt Ratio on New Code
  

1. New test coverage with Metrics
   
• Coverage on New Code
  
• Unit Test Duration

The first one was used everywhere except Referencedata service. I already changed this so configuration for all services is consistent. 

It leaves a gap. We don't use metric for unit test duration. In New test coverage it is set to 15ms. I doubt that this is correct setting for now as in every service we have many tests that wouldn't pass (mainly because of rest template and IO tests).

Thoughts?

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAAdp53z8zNzX1ky3X85_BEUsRx0dEpQQ-W63bCAG9Q6x_3ZUGg%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Paweł Albecki
Software Developer
palb...@soldevelo.com

[Paweł Gesek]

0% gate seems a bit steep, I mean I'm all for DRY, but a little duplication here or there can happen. I would say anything below 1% is good enough.

Regards,

Paweł

[mkwiatkowski]

Hi all,

I want to revive this discussion and ask simple question: is it a good idea to restrict our quality gate a bit? On our Learning Session we agreed that the gate should fail on any new code smell, if we won't be fixing them we can always remove issue directly from the Sonar. Now when we are introducing code smells we don't have any info if we don't check Sonar manually.

Regards,

Mateusz

[Paweł Albecki]

Hi Mateusz,

I do not think it is the best idea to have sonar build failed just because there are some small code smells. It will be great, though, if you find a way to have some notifications for developers who introduced that smell and maybe to Slack #dev channel as well.

Best regards,

Paweł

--

You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/202a0dc7-17d5-473e-84f5-c01b379766b0%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Paweł Albecki
Software Developer
palb...@soldevelo.com

[Mateusz Kwiatkowski]

Hi Pawel,

I don't  think it is that bad, Sonar build does not block other whole workflow so changes will be deployed etc., than you can disable issue on Sonar and next Sonar build won't fail,  but you are right, some notification will suffice. 

Regards,

Mateusz

[Paweł Gesek]

+1 to breaking the build on code smells - these warnings usually have little sense if they don't break the builds.

Regards,

Paweł

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAHq-FDPR%3DDVwsk_Ve93%2BVCfsReR2GUwZhk78HTdXimp%3DA56yeQ%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Paweł Gesek
Technical Project Manager
pge...@soldevelo.com / +48 690 020 875

[Paweł Albecki]

Code smell is just tech debt. I am also for fixing them immediately as they arise but I am wondering if dealing with things like small code duplication, cyclomatic complexity or remove usage of some deprecated class is more important than deliver on time. 

We also have some sonar rules that doesn't make sense like: Annotate the interface with the @FunctionalInterface annotation. Not every interface with one method is suppose to be used as functional interface.

[Paweł Gesek]

If we have rules that we don't intend to follow we should disable them, before we set the gates to fail.

The argument of sacrificing quality to achieve deadlines is a slippery slope - for example Scrum forbids it during a sprint. We all know how it goes when there is a rush towards a release, but all debt has to be paid sooner or later.

Regards,

Paweł

[Paweł Albecki]

One more thing, we still don't have quality gate for duplications. Łukasz, will you add this or I should do it?

Regards,

Paweł

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CADt-Nu1b3RKRhTAwiLDgVwPGVvbZNY%3D%3D51A__7tuPeD3iV3Mjg%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Paweł Albecki
Software Developer
palb...@soldevelo.com

[Łukasz Lewczyński]

If we agreed to add this quality gate, feel free to add it.

- Lukasz

[Paweł Albecki]

I think there was an agreement that 1% is not too low. I set this value for now, we can always lower it later.

- Paweł 

--

Paweł Albecki
Software Developer
palb...@soldevelo.com

[brandon.bowersox-johnson]

+1 for Paweł Gesek's comments: 

• If we have rules we do not intend to follow, we should disable those.
• When it comes to Code Smells and Sonar alerts, we should not sacrifice quality to achieve deadlines. If we let a few months go by sacrificing quality or ignoring sonar alerts, we would have tons of code smells in there. At that point, we would have no memory of all those parts of the codebase we were working on. It's much cheaper to get alerts and fix it right away, hopefully within minutes after we write the code itself.

[Mateusz Kwiatkowski]

Hi,

I've restricted Sonar quality gate to fail on any code smell, to get it "work" on UI I had to disable checks for:
* trailing spaces - because of problems with License Header, we have ticket for that: OLMIS-3964,

* using strict in JS files - we are using it in every JS file,

* to many parameters in JS functions - the problem is with injecting more than 7 services or other parameters to controllers etc. which we can't avoid, but we are also loosing check for number of parameters in regular functions,

* function names should use cammelCase - we have functions like ProofOfDeliveryRepository or FacilityListController which are starting with upper case letter, we could rename them and keep original names while declaring those components (angular.module(some-module).controller('FacilityListController', facilityListController))

Any opinions on this? Should we keep disabled checks or somehow resolve some them?

Best Regards,
Mateusz

--

You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/8aa75aef-c803-45ef-85e3-e78468f0f61a%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Nikodem Graczewski]

Hi Mateusz,

I think we should rethink point 3. If we have a controller that uses that many things it probably indicates that it is trying to do too many things. I would, perhaps, turn that check back on and see how it goes with the new code (I believe the old code wouldn't be caught unless we modify it, right?).
About point 4, this should be resolved once we get our hands on ES6 and JavaScript classes.

Best regards,
Nikodem

Nikodem Graczewski

Software Developer

ngrac...@soldevelo.com

On Wed, Mar 7, 2018 at 1:45 PM, Mateusz Kwiatkowski <mkwiat...@soldevelo.com> wrote:

Hi,

I've restricted Sonar quality gate to fail on any code smell, to get it "work" on UI I had to disable checks for:
* trailing spaces - because of problems with License Header, we have ticket for that: OLMIS-3964,

* using strict in JS files - we are using it in every JS file,

* to many parameters in JS functions - the problem is with injecting more than 7 services or other parameters to controllers etc. which we can't avoid, but we are also loosing check for number of parameters in regular functions,

* function names should use cammelCase - we have functions like ProofOfDeliveryRepository or FacilityListController which are starting with upper case letter, we could rename them and keep original names while declaring those components (angular.module(some-module).controller('FacilityListController', facilityListController))

Any opinions on this? Should we keep disabled checks or somehow resolve some them?

Best Regards,
Mateusz

On Thu, Feb 22, 2018 at 9:12 PM, brandon.bowersox-johnson <brandon.bowersox-johnson@villagereach.org> wrote:

+1 for Paweł Gesek's comments: 

• If we have rules we do not intend to follow, we should disable those.
• When it comes to Code Smells and Sonar alerts, we should not sacrifice quality to achieve deadlines. If we let a few months go by sacrificing quality or ignoring sonar alerts, we would have tons of code smells in there. At that point, we would have no memory of all those parts of the codebase we were working on. It's much cheaper to get alerts and fix it right away, hopefully within minutes after we write the code itself.

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/8aa75aef-c803-45ef-85e3-e78468f0f61a%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAHq-FDP0P32TmUQvB2XqjQbZT8YMfo9SGkCvPHNbKm-a_0o%2BuA%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

[Paweł Albecki]

Hi,

Few weeks ago I enabled Quality Gate for Duplicated Lines on New Code and set it to 1% for trial. It didn't cause any issues for Java code, I think we should even decrease value to 0% duplicated lines. Although, this doesn't apply to Angular JS code. Sonar currently shows as duplicated elements that can and sometimes even should be duplicated like: array with dependencies for $inject, method calls. That issue exists for some routes.js files in referencedata-ui (role routes) and stockmanagement-ui (adjustment creation routes). Static analyzer treats second param (config object) for $stateProvider.state as duplication because it's almost the same for few files. The only solution I have in mind is provide whole config object for $stateProvider.state method by some provider but this doesn't sound very clean. 

I recently removed real code duplication from adjustment creation routes resolve properties, by extracting code to factories. Despite this, gate failed for reasons described above. Because of this I increased permitted duplicated lines on new code setting to 5% until the solution will be found how to deal with this problem in clean way. 

I'm looking forward for your suggestions.

Regards,

Paweł

On Wednesday, March 7, 2018 at 2:20:44 PM UTC+1, Nikodem Graczewski wrote:

Hi Mateusz,

I think we should rethink point 3. If we have a controller that uses that many things it probably indicates that it is trying to do too many things. I would, perhaps, turn that check back on and see how it goes with the new code (I believe the old code wouldn't be caught unless we modify it, right?).
About point 4, this should be resolved once we get our hands on ES6 and JavaScript classes.

Best regards,
Nikodem

Nikodem Graczewski

Software Developer

ngrac...@soldevelo.com

On Wed, Mar 7, 2018 at 1:45 PM, Mateusz Kwiatkowski <mkwiat...@soldevelo.com> wrote:

Hi,

I've restricted Sonar quality gate to fail on any code smell, to get it "work" on UI I had to disable checks for:
* trailing spaces - because of problems with License Header, we have ticket for that: OLMIS-3964,

* using strict in JS files - we are using it in every JS file,

* to many parameters in JS functions - the problem is with injecting more than 7 services or other parameters to controllers etc. which we can't avoid, but we are also loosing check for number of parameters in regular functions,

* function names should use cammelCase - we have functions like ProofOfDeliveryRepository or FacilityListController which are starting with upper case letter, we could rename them and keep original names while declaring those components (angular.module(some-module).controller('FacilityListController', facilityListController))

Any opinions on this? Should we keep disabled checks or somehow resolve some them?

Best Regards,
Mateusz

On Thu, Feb 22, 2018 at 9:12 PM, brandon.bowersox-johnson <brandon.bowe...@villagereach.org> wrote:

+1 for Paweł Gesek's comments: 

• If we have rules we do not intend to follow, we should disable those.
• When it comes to Code Smells and Sonar alerts, we should not sacrifice quality to achieve deadlines. If we let a few months go by sacrificing quality or ignoring sonar alerts, we would have tons of code smells in there. At that point, we would have no memory of all those parts of the codebase we were working on. It's much cheaper to get alerts and fix it right away, hopefully within minutes after we write the code itself.

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/8aa75aef-c803-45ef-85e3-e78468f0f61a%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAHq-FDP0P32TmUQvB2XqjQbZT8YMfo9SGkCvPHNbKm-a_0o%2BuA%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.