adoroszlai commented on a change in pull request #419:
URL: https://github.com/apache/incubator-ratis/pull/419#discussion_r576069188
##########
File path: .github/workflows/post-commit.yml
##########
@@ -151,3 +146,28 @@ jobs:
- name: Delete temporary build artifacts
run: rm -rf ~/.m2/repository/org/apache/ratis
if: always()
+ sonar:
+ name: sonar
+ runs-on: ubuntu-18.04
+ if: github.repository == 'apache/incubator-ratis' && github.event_name !=
'pull_request'
Review comment:
Thanks @softgitron for the review.
> Sonar analysis will still fail couple of seconds later in the sonar.sh
file due to missing environment variables so it is only matter of which error
message you will get.
With this check the workflow simply skips the `sonar` job in forks. If we
omit the check, the workflow will fail with an error. I don't think these two
are the same, as in the second case the error is expected and needs to be
worked around mentally. So I'd keep this check.
> This makes usage of the sonar checks annoying on the forks, because line
must be always patched away. (I'm not expecting that forking will be especially
common for an open source project, but still improvement)
Forks are common for development, but I think only very few would register
their forks in Sonar. What is the benefit or use case for doing that?
> Just to remind, after Ratis graduates as TLP sonar.sh should be also
updated with new `-Dsonar.organization` and `-Dsonar.projectKey`
I'm not very familiar with registering projects in Sonarcloud, but I don't
think they are automatically renamed there. Project key seems to be
persistent. Apache Ozone for example still goes by
[`hadoop-ozone`](https://sonarcloud.io/dashboard?id=hadoop-ozone).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
