Skip to content

.github: run Sonar Check only on PRs from apache/cloudstack branches #8058

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.

Sign up for GitHub

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

Merged
shwstppr merged 1 commit into from
Oct 10, 2023
Merged

.github: run Sonar Check only on PRs from apache/cloudstack branches #8058

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/sonar-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ concurrency:

jobs:
build:
if: github.repository == 'apache/cloudstack'
if: github.repository == 'apache/cloudstack' && github.event.pull_request.head.repo.full_name == github.repository
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Oct 9, 2023

Choose a reason for hiding this comment

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

Suggested change
if: github.repository == 'apache/cloudstack' && github.event.pull_request.head.repo.full_name == github.repository
if: github.event.pull_request.head.repo.full_name == github.repository

i.e. people may run sonar on their own forks?

Copy link
Copy Markdown
Member Author

@weizhouapache weizhouapache Oct 9, 2023

Choose a reason for hiding this comment

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

if so, users need to configure sonar token in github project.

For example, if someone creates a PR in shapeblue (forked) repositories, currently the sonar check is skipped.
But with this change, sonar check will not be skipped, but fail (as sonar token does not exist in project settings).

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Oct 9, 2023

Choose a reason for hiding this comment

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

but only if they create a PR from their own repo to their own repo, so they have control on whether they create the token or not.

Copy link
Copy Markdown
Member Author

@weizhouapache weizhouapache Oct 9, 2023

Choose a reason for hiding this comment

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

yes. so between these 2 options with forks

  • enable PR sonar check by default. users need to disable it by code changes if they do not have sonar token.
  • disable PR sonar check by default. users need to enable it by code changes if they have sonar token.

the PR sonar check was disabled on forks by #7199
we can change the behaviour if needed

Copy link
Copy Markdown
Member Author

@weizhouapache weizhouapache Oct 9, 2023

Choose a reason for hiding this comment

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

ps: the PR codecov check was disabled on forks by #7177

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Oct 9, 2023

Choose a reason for hiding this comment

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

but in both cases the code may not be merged, inconvenient :(

Copy link
Copy Markdown
Member Author

@weizhouapache weizhouapache Oct 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Right...we have to choose one of the two options.
Let's assume that most users do not have sonar token, so we run sonar check only on apache/cloudstack repository. Make sense ?

name: Sonar JaCoCo Coverage
runs-on: ubuntu-22.04
steps:
Expand Down