Avoid maybeSonarHome.get #7
Conversation
|
|
||
| // Update the external properties file if the sonarUseExternalConfig is set to true. | ||
| if (sonarUseExternalConfig.value) | ||
| updatePropertiesFile(baseDirectory.value, SonarExternalConfigFileName, version.value) | ||
|
|
||
| val args = sonarScannerArgs(sonarUseExternalConfig.value, sonarProperties.value, version.value) | ||
| //Allow to set sonar properties via system properties: [https://docs.sonarqube.org/display/SONAR/Analysis+Parameters] |
There was a problem hiding this comment.
That's a nice improvement 👍
I'm wondering whether it would be better to populate the system properties first and then merge those with sonarProperties - just in case if you want to overwrite any system settings on a project level.
There was a problem hiding this comment.
For my opinion, system properties are much preferable than project-files properties. In that case you can set project name or other properties for different branches without any files changing.
There was a problem hiding this comment.
Ok, that's a valid point, especially when you e.g. run an analysis on a branch and you want to overwrite some settings in your build pipeline.
|
|
||
| // Update the external properties file if the sonarUseExternalConfig is set to true. | ||
| if (sonarUseExternalConfig.value) | ||
| updatePropertiesFile(baseDirectory.value, SonarExternalConfigFileName, version.value) | ||
|
|
||
| val args = sonarScannerArgs(sonarUseExternalConfig.value, sonarProperties.value, version.value) | ||
| //Allow to set sonar properties via system properties: [https://docs.sonarqube.org/display/SONAR/Analysis+Parameters] | ||
| val ssonarProperties = sonarProperties.value ++ sys.env.filterKeys(_.startsWith("sonar.")) |
There was a problem hiding this comment.
I'd probably rename this to something like e.g. mergedSonarProperties.
|
|
||
| // Update the external properties file if the sonarUseExternalConfig is set to true. | ||
| if (sonarUseExternalConfig.value) | ||
| updatePropertiesFile(baseDirectory.value, SonarExternalConfigFileName, version.value) | ||
|
|
||
| val args = sonarScannerArgs(sonarUseExternalConfig.value, sonarProperties.value, version.value) | ||
| //Allow to set sonar properties via system properties: [https://docs.sonarqube.org/display/SONAR/Analysis+Parameters] | ||
| val mergedSonarProperties = sonarProperties.value ++ sys.env.filterKeys(_.startsWith("sonar.")) |
There was a problem hiding this comment.
Perhaps you were after sys.props instead of sys.env?
There was a problem hiding this comment.
Ouch, yes. It is my mistake. I talk about system properties. I will correct it .
|
Would you also be able to update README and mention about system properties? An example of how to run |
|
That's great, thanks a lot for your contribution @yarosman! On a side note, I'm currently working on SonarQube 6.7.1 LTS support in my fork of the sonar-scala plugin, so if you're interested, check out https://github.com/mwz/sonar-scala/tree/sonar-671-lts and https://github.com/mwz/sonarqube-scala-docker in a little while once the new version is ready. |
avoid optionality for sonarHome path variable.
allow to set sonar system properties