reviewdog / action-staticcheck

🐶 Run staticcheck with reviewdog on pull requests to improve code review experience.
https://github.com/marketplace?type=actions&query=reviewdog
MIT License
18 stars 2 forks source link

Allow go.mod deps from private repositories #14

Closed kstiehl closed 3 years ago

kstiehl commented 3 years ago

Currently this action can't be used as soon as you have at least one go dependency which requires authentication.

Currently the only way in go to allow fetching private repositories is by adding an URL patch to your global gitconfig and setting the GOPRIVATE var.

The github action runner passes the GOPRIVATE variable correctly to the docker container, but the gitconfig of the runner has no effect on the go installation inside the container.

With this change it is now possible to apply git url patches using an environment variables prefixed with GIT_URL_OVERRIDE

In our setup we use the action like this now

  - uses: ourOrg/action-staticcheck@master
     name: Staticcheck
     env:
       GOPRIVATE: github.com/ourOrg/privateRepo
       GIT_URL_OVERRIDE: https://${{ secrets.GO_MODULE_TOKEN }}:x-oauth-basic@github.com;https://github.com
     with:
       reporter: github-pr-review
       filter_mode: nofilter

I also changed the /bin/sh to /bin/bash since /bin/sh is a link to a bash on most linux systems, except for ubuntu where it is a dash shell. So /bin/sh is no guarantee for what shell will be used. (Also this change requires the bash expansion features)

kstiehl commented 3 years ago

This should fix #9 since it then allows the use of private repositories

haya14busa commented 3 years ago

Thank you for your work!

I have 2 opinions.

  1. I'm considering using composite run action instead of docker action for this action-staticcheck as well, similar to https://github.com/reviewdog/action-golangci-lint. Then, users can setup anything including private repo config outside this action. Do you think you can work on this docker->composite run migration instead? WDYT?
  2. If we still want to pass the env variable, I'd like to have 2 environment variable for GIT_URL_OVERRIDE instead of depending on awk and bash expansion internally.
kstiehl commented 3 years ago

Hi @haya14busa Thnx for your feedback.

I'm considering using composite run action instead of docker action for this action-staticcheck as well, similar to https://github.com/reviewdog/action-golangci-lint. Then, users can setup anything including private repo config outside this action. Do you think you can work on this docker->composite run migration instead? WDYT?

That's actually even better. It totally makes sense to migrate this action to a composite action. If you don't mind I will start working on that at the weekend.

If we still want to pass the env variable, I'd like to have 2 environment variable for GIT_URL_OVERRIDE instead of depending on awk and bash expansion internally.

I think that we won't need the env anymore once the migration to a composite action is done plus awk was only an option since it was present in the ubuntu container anyways. We will get rid of this completely then

haya14busa commented 3 years ago

Yes! I'd appriciate it if you can work on the migration!

kstiehl commented 3 years ago

Closing PR in favor of the mentioned changes