Remove condition on pull_request events #6

sawy · 2019-09-20T18:51:47Z

Introduces a way to run the scanner action on pull_request
events by setting the SCAN_PULL_REQUEST environment flag.

simonbrandhof · 2019-10-02T14:39:20Z

README.md

@@ -46,6 +46,10 @@ jobs:

 - `SONAR_TOKEN` – **Required** this is the token used to authenticate access to SonarCloud. You can generate a token on your [Security page in SonarCloud](https://sonarcloud.io/account/security/). You can set the `SONAR_TOKEN` environment variable in the "Secrets" settings page of your repository.

+### Environment variables
+
+- `SCAN_PULL_REQUEST` – **Optional** Flag to run analysis on `pull_request` events. Scans on pull requests are disabled by default as the action expects to be triggered by push events. 


Where is this variable used?

I messed up the master branch on our side accidentally. Sorry!

I've reverted it to the relevant commit; The variable is used used in entrypoint.sh.

simonbrandhof · 2019-10-02T19:43:50Z

Is the goal to trigger an analysis on the creation of a pull request, so that there's no need to push a new commit?

ZeevKatz · 2019-11-04T20:49:56Z

entrypoint.sh

@@ -17,7 +17,7 @@ if [[ -f "build.gradle" ]]; then
  exit 1
 fi

-if [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then
+if [[ -z "${SCAN_PULL_REQUEST}" || "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then
  EVENT_ACTION=$(jq -r ".action" "${GITHUB_EVENT_PATH}")
  if [[ "${EVENT_ACTION}" != "opened" ]]; then
 	  echo "No need to run analysis. It is already triggered by the push event."


@sawy @simonbrandhof Why is this block needed?
You can let the users decide to run the analysis whenever they want by using the if conditional.

You can use the if conditional to prevent a step from running unless a condition is met. You can use any supported context and expression to create a conditional.

See this for more details

@sawy @simonbrandhof Why is this block needed?

I don't know that. The scripts seems to assume that there is a workflow in place that runs on push. We have workflows that only listen to pull_request events and therefore never run the analysis. Hence the new flag to overwrite the behavior.

That is a safeguard to prevent analysis of branch when a pull request exists. To be honest that was put in place by fear of having users misunderstanding how branches and pull requests work in SonarCloud. Indeed a branch can be analysed in isolation of its associated pull request.

Removing this condition, and the suggested SCAN_PULL_REQUEST variable, is an option that could make things simpler. It would be up to users to correctly configure their workflow.

Does that make sense?

@simonbrandhof could you elaborate a bit more? I think I'm part of the misunderstanding users 😟, sorry.
If we open a PR, the checkout action will AFAIK (per default) checkout a merge commit that may differ from the branch HEAD. In such case why analysis should be bypassed?

@loicalbertin it doesn't actually make much sense at all, but it seems @simonbrandhof doesn't want to change anything either. If you want to have your pull requests scanned properly, try pr-fix branch on my fork. We've been using it for quite some time now and haven't noticed any issues neither on SonarCloud side, nor on GitHub itself. Everything's working fine.

You can include it in your github workflow the following way:

- name: SonarCloud Scan uses: hakimio/sonarcloud-github-action@pr-fix env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}

sawy · 2019-11-04T22:23:57Z

Is the goal to trigger an analysis on the creation of a pull request, so that there's no need to push a new commit?

The goal is to run the action on pull_request events as well, even though no push is involved at all.

hakimio · 2019-11-15T08:55:36Z

@simonbrandhof Any reason this can not be merged? We need this functionality as well.

simonbrandhof · 2019-11-15T09:41:55Z

@simonbrandhof Any reason this can not be merged? We need this functionality as well

See #6 (comment). Could that help?

hakimio · 2019-11-15T10:09:37Z

@simonbrandhof I understand the reasoning behind this check but I also think it should be completely optional.

hakimio · 2019-11-15T10:11:01Z

entrypoint.sh

@@ -17,7 +17,7 @@ if [[ -f "build.gradle" ]]; then
  exit 1
 fi

-if [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then
+if [[ -z "${SCAN_PULL_REQUEST}" || "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then


I'm pretty sure it should be && instead of || condition. Otherwise any additional commits to the PR are not analyzed.

SonarCloud Github Action currently does not support PullRequest runs: SonarSource/sonarcloud-github-action#6

* Create sonar_cloud_scan.yml * Update sonar-project.properties SonarCloud Github Action currently does not support PullRequest runs: SonarSource/sonarcloud-github-action#6 * Update .travis.yml Disable SonarCloud in Travis

FahdW · 2019-12-17T17:48:33Z

If I only have a scan on pull_requests, when a someone pushes a new commit to a PR you would need to trigger on pushes and NOT have a PR trigger alone. Since sonarcloud will decide for itself whether it needs to scan or not. Which comes a little opinionated

lechen26 · 2020-02-02T12:21:34Z

any news about it?

hakimio · 2020-02-02T18:54:18Z

@lechen26 no, but you can use my fork to have PRs scanned properly: #6 (comment)

Removes restriction on EVENT_ACTION and GITHUB_EVENT_NAME, to allow analysis on any pull request event. It's up to the users to correctly configure their workflow.

sawy · 2020-02-10T23:25:42Z

@simonbrandhof and @ZeevKatz, I've updated this PR to simply remove the condition as suggested and implemented by @hakimio.

ZeevKatz · 2020-02-16T13:05:02Z

@sawy 😜Approved! Now seriously, why we're waiting with this merge?

abusi · 2020-02-19T13:54:46Z

It would be so nice to have this merged ...

hakimio · 2020-02-19T14:00:18Z

@simonbrandhof There are quite a few people who want this merged. What's your final decision? Merge or just close?

ZeevKatz · 2020-03-01T13:45:56Z

@simonbrandhof Please update what's your decision.

westonkjones · 2020-03-11T03:16:54Z

We also need this feature. Interested to hear what your thoughts are. Due to limited resources we are scanning locally when a commit is made and then we are requiring analysis to pass before a PR can be merged

westonkjones · 2020-03-11T03:48:43Z

If I only have a scan on pull_requests, when a someone pushes a new commit to a PR you would need to trigger on pushes and NOT have a PR trigger alone. Since sonarcloud will decide for itself whether it needs to scan or not. Which comes a little opinionated

@FahdW This is a really good point, and was worth checking out. I am currently using @sawy 's fork and analysis is being ran when I make a push to the branch containing the PR. So there is no need for an extra step to run on push. In fact, something similar to this works exactly as I am expecting

on:
  pull_request:
    branches: [master, develop]
jobs:
  analyze:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@master
      - name: SonarCloud Scan
        uses: brudi/sonarcloud-github-action@master
        env:
          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          SCAN_PULL_REQUEST: true

edouardr · 2020-03-13T11:30:40Z

Hi guys, would require this too :). My scan on push to PR gets ignored, which is a bit annoying as it makes my PR checks invalid.

soham308 · 2020-03-23T15:04:58Z

Guys This may look like a nice to have feature. But it will very much be required for us. Kindly push it through 😄

luczsoma · 2020-03-27T00:07:16Z

Hi guys, it would be very nice if this would finally be merged into master. We have been waiting for a half year now. Thank you in advance.

simonbrandhof · 2020-03-30T18:13:05Z

This pull request significantly changed since the initial contribution. Dropping the filter on events pull_request sounds to be the best solution. In order to document how to achieve the current behavior, the pull request #10 has been created. Does someone want to have a look?
Thanks in advance.

simonbrandhof · 2020-03-30T18:38:26Z

Merged into master, through #10.

kenhuang · 2020-05-01T00:31:04Z

can we have a new tag release for this change?

simonbrandhof · 2020-05-04T08:17:15Z

@kenhuang Sure. It will be done in the next few hours.

simonbrandhof reviewed Oct 2, 2019

View reviewed changes

ZeevKatz reviewed Nov 4, 2019

View reviewed changes

hakimio reviewed Nov 15, 2019

View reviewed changes

mrck added a commit to bevrand/bevrand that referenced this pull request Nov 24, 2019

Update sonar_cloud_scan.yml

c0b1569

SonarCloud Github Action currently does not support PullRequest runs: SonarSource/sonarcloud-github-action#6

remove condition on pull_request events

d3581c3

Removes restriction on EVENT_ACTION and GITHUB_EVENT_NAME, to allow analysis on any pull request event. It's up to the users to correctly configure their workflow.

ZeevKatz approved these changes Feb 16, 2020

View reviewed changes

kundkingan approved these changes Mar 27, 2020

View reviewed changes

simonbrandhof changed the title ~~Allow to scan on pull_request events~~ Remove condition on pull_request events Mar 30, 2020

simonbrandhof self-assigned this Mar 30, 2020

simonbrandhof mentioned this pull request Mar 30, 2020

Accept triggering from GitHub event "pull_request" #10

Merged

simonbrandhof closed this Mar 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove condition on pull_request events #6

Remove condition on pull_request events #6

sawy commented Sep 20, 2019

simonbrandhof Oct 2, 2019

sawy Oct 2, 2019

simonbrandhof commented Oct 2, 2019

ZeevKatz Nov 4, 2019

sawy Nov 4, 2019

simonbrandhof Nov 15, 2019

loicalbertin Jan 17, 2020

hakimio Jan 17, 2020

sawy commented Nov 4, 2019 •

edited

Loading

hakimio commented Nov 15, 2019

simonbrandhof commented Nov 15, 2019

hakimio commented Nov 15, 2019 •

edited

Loading

hakimio Nov 15, 2019

FahdW commented Dec 17, 2019

lechen26 commented Feb 2, 2020

hakimio commented Feb 2, 2020

sawy commented Feb 10, 2020

ZeevKatz commented Feb 16, 2020

abusi commented Feb 19, 2020

hakimio commented Feb 19, 2020

ZeevKatz commented Mar 1, 2020 •

edited

Loading

westonkjones commented Mar 11, 2020

westonkjones commented Mar 11, 2020 •

edited

Loading

edouardr commented Mar 13, 2020 •

edited

Loading

soham308 commented Mar 23, 2020

luczsoma commented Mar 27, 2020

simonbrandhof commented Mar 30, 2020

simonbrandhof commented Mar 30, 2020 •

edited

Loading

kenhuang commented May 1, 2020

simonbrandhof commented May 4, 2020

Remove condition on pull_request events #6

Remove condition on pull_request events #6

Conversation

sawy commented Sep 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonbrandhof commented Oct 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sawy commented Nov 4, 2019 • edited Loading

hakimio commented Nov 15, 2019

simonbrandhof commented Nov 15, 2019

hakimio commented Nov 15, 2019 • edited Loading

Choose a reason for hiding this comment

FahdW commented Dec 17, 2019

lechen26 commented Feb 2, 2020

hakimio commented Feb 2, 2020

sawy commented Feb 10, 2020

ZeevKatz commented Feb 16, 2020

abusi commented Feb 19, 2020

hakimio commented Feb 19, 2020

ZeevKatz commented Mar 1, 2020 • edited Loading

westonkjones commented Mar 11, 2020

westonkjones commented Mar 11, 2020 • edited Loading

edouardr commented Mar 13, 2020 • edited Loading

soham308 commented Mar 23, 2020

luczsoma commented Mar 27, 2020

simonbrandhof commented Mar 30, 2020

simonbrandhof commented Mar 30, 2020 • edited Loading

kenhuang commented May 1, 2020

simonbrandhof commented May 4, 2020

sawy commented Nov 4, 2019 •

edited

Loading

hakimio commented Nov 15, 2019 •

edited

Loading

ZeevKatz commented Mar 1, 2020 •

edited

Loading

westonkjones commented Mar 11, 2020 •

edited

Loading

edouardr commented Mar 13, 2020 •

edited

Loading

simonbrandhof commented Mar 30, 2020 •

edited

Loading