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

[BOP-54] Run CI on a PR #6

Merged
merged 4 commits into from
Nov 2, 2023
Merged

[BOP-54] Run CI on a PR #6

merged 4 commits into from
Nov 2, 2023

Conversation

nwneisen
Copy link
Collaborator

@nwneisen nwneisen commented Nov 2, 2023

What this PR does

Runs CI for vet, unit-tests, and build when a PR is created or updated.

https://mirantis.jira.com/browse/BOP-54

Testing

Tested by merging the PR into my fork of the repo and then opening a PR. Made sure all of the steps ran and passed when changing something other than a MD file.

Notes for Reviewers

These jobs will pretty much be a copy and paste to the other BOP repos. Take a look, checkout the readme, let me know if anything needs to be changed or clarified before copying it to the other repos.

Copy link
Collaborator

@ranyodh ranyodh left a comment

Choose a reason for hiding this comment

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

Just one small change. Other, looks awesome!

Regarding caching:
It would great if we can figure out dependency caching as well. But, it is not required right now. We can create separate ticket for it.

To be more specific, I referring to the build step which runs make build. The docker build step does not have access to the cache that actions/setup-go@v4 action generates.

.github/workflows/PR.yml Show resolved Hide resolved
@nwneisen
Copy link
Collaborator Author

nwneisen commented Nov 2, 2023

@ranyodh caching seems like it always seems to be a little tricky. I'll create another ticket to look at doing it by itself and we can get CI running on our PRs with these changes. The docker-build command is also running tests which I don't think we want in local dev or in the CI step. I will take a look at all the Makefile commands closer and we can start to modify them.

@ranyodh
Copy link
Collaborator

ranyodh commented Nov 2, 2023

caching seems like it always seems to be a little tricky. I'll create another ticket to look at doing it by itself and we can get CI running on our PRs with these changes.
Yes, especially inside the docker. Lets do that with another ticket.

The docker-build command is also running tests which I don't think we want in local dev or in the CI step. I will take a look at all the Makefile commands closer and we can start to modify them.

Yes, these make commands comes auto-generated with the operator-sdk. We can modify them as we go.

@nwneisen nwneisen merged commit 108e41c into Mirantis:main Nov 2, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants