Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GHA] Do not run sonarcloud on forks #1142

Closed
wants to merge 5 commits into from

Conversation

hazendaz
Copy link
Contributor

Noticed on my fork that sonar cloud was trying to run and of course failing, add line to prevent it from running on anyone but owner of repository. You may want this partially in other cases such as gh-pages but that one was not failing for me.

Copy link

codecov bot commented Dec 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4b5358a) 98.27% compared to head (b1831eb) 98.27%.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #1142   +/-   ##
=========================================
  Coverage     98.27%   98.27%           
  Complexity      567      567           
=========================================
  Files            55       55           
  Lines          1214     1214           
  Branches        105      105           
=========================================
  Hits           1193     1193           
  Misses           10       10           
  Partials         11       11           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -8,6 +8,7 @@ on:

jobs:
build:
if: github.repository_owner == 'damianszczepanik'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. leave comments in the code why is that
  2. with this change there is no need to have branch limitation in line 5-6

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added variation of the first comment above 'if'. Adjusted the original. I think you do still want it only running against master, that is unless you are looking at pull requests. I can only speak for repos I work on and all simply run on master and nothing else. If you are looking to more broadly look at those then sure, will remove that.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general - agree with you. My experience is that sonar quite frequently has problem with authentication because of token that is lost somehow. Quite often I did not notice the problem with sonar token and for a long time sonar was not working. Having this on PR (even for myself ) gives opportunity to find such issues earlier :)
Best would be to process sonar against every PR to see tech debt earlier but I was not able to do that :(

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this allows me to run sonar on fix-branch before fix goes to master so I can perform the validation and confirm the fix before it goes to master :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@damianszczepanik Per https://docs.sonarsource.com/sonarqube/latest/devops-platform-integration/github-integration/, unless you are a paying customer ($160 a year), its not possible to run on pull requests. So I've adjusted this a bit to indicate that and point to the documentation.

@hazendaz hazendaz force-pushed the GHA branch 2 times, most recently from 821278d to 44a9e77 Compare December 31, 2023 19:31
@hazendaz
Copy link
Contributor Author

Guess more work needed to get sonar part working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants