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

Add pull request workflow #1359

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

HarrisonWAffel
Copy link
Contributor

Problem

#1351 missed a workflow for pull requests, as such we are not building and packaging the provider before allowing PRs to be merged

Solution

Add a pull request workflow which builds and packages the provider for each PR. This PR does not include the acceptance tests, as additional effort needs to be put into how we can modernize those tests such that they can work against a more modern version of rancher.

Testing

I've tested this workflow on my personal fork

Actions run: https://github.com/HarrisonWAffel/terraform-provider-rancher2/actions/runs/9571691853/job/26389359180

@HarrisonWAffel HarrisonWAffel requested review from jiaqiluo and a team June 18, 2024 20:39
Copy link
Collaborator

@snasovich snasovich left a comment

Choose a reason for hiding this comment

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

LGTM at a glance but some observations/questions:

  1. On Drone we ran make docker-build and make build-rancher is basically equivalent of it minus Docker. Why do we need to run make package-rancher?
  2. Any specific reason we're running on ubuntu-20.04 vs. ubuntu-latest?

@snasovich snasovich requested a review from a team June 18, 2024 20:56
@HarrisonWAffel HarrisonWAffel force-pushed the pull-request-workflow branch from 55c8c93 to f0da508 Compare June 18, 2024 21:04
@HarrisonWAffel
Copy link
Contributor Author

I've updated to ubuntu-latest, and removed make package-rancher. Initial thought process was to more closely mirror what release CI does, but if this step was not being done while running in drone I agree we should drop it.

Copy link
Member

@jiaqiluo jiaqiluo left a comment

Choose a reason for hiding this comment

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

LGTM

@jiaqiluo jiaqiluo requested a review from snasovich June 18, 2024 21:31
@HarrisonWAffel HarrisonWAffel force-pushed the pull-request-workflow branch from f0da508 to d04a3bd Compare June 18, 2024 21:34
@HarrisonWAffel HarrisonWAffel force-pushed the pull-request-workflow branch 3 times, most recently from 29d66ab to 866fbbe Compare June 18, 2024 21:43
@HarrisonWAffel HarrisonWAffel force-pushed the pull-request-workflow branch from 866fbbe to 6efbc74 Compare June 18, 2024 21:43
Copy link
Collaborator

@snasovich snasovich left a comment

Choose a reason for hiding this comment

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

@HarrisonWAffel HarrisonWAffel merged commit 642464a into rancher:master Jun 18, 2024
1 check passed
jiaqiluo pushed a commit to jiaqiluo/terraform-provider-rancher2 that referenced this pull request Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants