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

ci: migrate to Github Actions, close #546 #750

Merged
merged 13 commits into from
Oct 5, 2020

Conversation

shinebayar-g
Copy link
Contributor

@shinebayar-g shinebayar-g commented Sep 2, 2020

Description

Let's migrate to Github Actions

Motivation and Context

See: #546 for more details

How Has This Been Tested?

I'm not Go developer, so I need your guidance to complete it. yml file syntax is pretty similar to travis ci. So I hope you'll be able to easily see what's going on.

Right now build is broken. Because of I don't know what's needed to fix. See builds here: https://github.com/shinebayar-g/oauth2-proxy/actions

Once we figure out travis ci replacement, we can move onto codeql action.

Checklist:

  • My change requires a change to the documentation or CHANGELOG.
  • I have updated the documentation/CHANGELOG accordingly.
  • I have created a feature (non-master) branch for my PR.

@shinebayar-g shinebayar-g requested a review from a team as a code owner September 2, 2020 03:27
@JoelSpeed
Copy link
Member

golangci/golangci-lint info checking GitHub for tag 'v1.24.0'
golangci/golangci-lint info found version: 1.24.0 for v1.24.0/linux/amd64
install: cannot create regular file '/bin/golangci-lint': Permission denied
##[error]Process completed with exit code 1.

This is more of a bash problem at the moment. Looks like we need to install golangci-lint into some other directory, I wonder if we can do that easily using the existing go releaser scripts

On a separate note, if we do this migration, I'd like to keep travis and github actions on initially, so that we can compare the results and make sure there are no issues with the new CI. Can we drop the travis ci drop from this PR please?

I think it might be good to use this opportunity to create three status checks, one for linting, one for the tests and one for build, ie, does the code compile, this may require some changes to the Makefile targets but shouldn't be too much. I think we basically want to get the output of make lint, make build and make test but stop them depending on each other.

@JoelSpeed
Copy link
Member

I think it might be good to use this opportunity to create three status checks, one for linting, one for the tests and one for build, ie, does the code compile, this may require some changes to the Makefile targets but shouldn't be too much. I think we basically want to get the output of make lint, make build and make test but stop them depending on each other.

@NickMeves @syscll @gargath WDYT of this suggestion?

@shinebayar-g
Copy link
Contributor Author

@JoelSpeed I've figured most of the workflow. Now I'm not sure what to do with ./after build script.

@JoelSpeed
Copy link
Member

Now I'm not sure what to do with ./after build script.

This needs to be in the test step, so I'd prefer if we had three separate jobs, which would give us three separate ticks right?

For the test one (make test), you need to have the before and after scripts in there too, so that it uploads the code coverage reports

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
test.sh Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

Just gonna wait for checks to pass, then ready to merge!

Thanks @shinebayar-g

@JoelSpeed JoelSpeed merged commit dc7dbc5 into oauth2-proxy:master Oct 5, 2020
Jing-ze pushed a commit to Jing-ze/oauth2-proxy that referenced this pull request Nov 19, 2024
Signed-off-by: sjcsjc123 <1401189096@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants