search

codacy / codacy-coverage-reporter-action

GitHub Action for the codacy-coverage-reporter
Other
58 stars 15 forks source link

feature: Use --skip by default to allow skipping report on pull requests CY-3472 #29

Closed lolgab closed 3 years ago

lolgab commented 3 years ago

The rationale here is that if you use the action you are forced to add the token from your secrets. Still it is a good default to skip CI on forks because you don't want to pass your secrets to forks.

Explanation We have a option in the binary to skip the coverage reporting when the token is not define.d This is handy if you have a public repository. This because generally you don’t want to give your Codacy token to people. So when you have a public repository you pass --skip and if the token is not defined ( the secret has not been passed ) the coverage reporter process will just return 0 without sending anything. Now, the action needs you to define the token as a mandatory parameter. So it can’t happen that you forget to add it. But if someone forks your repository the CI will fail because they don’t have the secrets.

machadoit commented 3 years ago

I am not sure if I understand... Then everyone will skip, no? What are the consequences of this change?

lolgab commented 3 years ago

I am not sure if I understand... Then everyone will skip, no? What are the consequences of this change?

Added a more thorough explanation in the PR description. Tell me if you still have doubts.

franciscodua commented 3 years ago

Seems like a risky default. It might break people's expectations and fail silently by default

machadoit commented 3 years ago

Thx for the explanation, it works, but I have to agree with @franciscodua, it is a risky default, and imo, a bug hazard. I would prefer that this action does exactly what it is suppose to do, which is, to send coverage.

How does it work in life usually? Shouldn't the user using the action be responsible to only run it when need it? Maybe they can settheir CI they have the option of certain steps only when it is not a fork, no? Like the options mentioned at https://github.community/t/stop-github-actions-running-on-a-fork/17965

So my suggestion would be to not introduce this feature, and let it crash when people don't have a token setup, if they don't do anything similar to the approaches mentioned above.

lolgab commented 3 years ago

Since this can cover bugs and there is a native way to skip actions on Github actions I'm closing this PR.