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

Fix PR from forked branch #103

Merged
merged 22 commits into from
Feb 14, 2024
Merged

Fix PR from forked branch #103

merged 22 commits into from
Feb 14, 2024

Conversation

benlee0423
Copy link

@benlee0423 benlee0423 commented Feb 8, 2024

Requirement from Issue 86:
On a pull request, the pipeline will build and run tests using the container on the runner in auto mode and sample test data. On merge to the main branch, the pipeline will build and push the Docker image to DockerHub.

What is included in this PR:

  1. Fix action launch on PR from forked branch
  2. Start and Stop Runner is disabled for X86 build.
  3. Add separate workflow for X86 build, and this is manual build.

What is NOT included in this PR:

  1. New build-and-test action is just place holder, and it will be replaced with build and run tests in issue 89 later.

Note: PR from forked branch needs to start/stop Runner manually for security reason all time.

Copy link
Member

@arpita0911patel arpita0911patel left a comment

Choose a reason for hiding this comment

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

Thank you @benlee0423 for working on these CI pipeline changes. Great work indeed!! I'm reviewing it now and will let you know if anything. Additional testing will be done once the PRs are submitted using forked branches. Also adding @hellkite500 and @ZacharyWills for final review.

@hellkite500
Copy link
Collaborator

Can you clarify at a high level what these workflow changes do when a PR is created?

From what I can tell, these are still attempting to do a complete end-to-end build, push, test workflow for every PR, and still requires manual engagement of the runner to perform any testing of a PR from an external source?

@benlee0423
Copy link
Author

Can you clarify at a high level what these workflow changes do when a PR is created?

See updated description of this PR, and requirement section.

@benlee0423
Copy link
Author

these are still attempting to do a complete end-to-end build, push, test workflow for every PR

No, on PR it will do build and test ( with auto-mode running with fixed input data).
New action is just place holder for now, because setting push: false in action does not create docker image on the Runner, as a result all subsequent builds failed. push: false will be replaced with load: true.

@benlee0423
Copy link
Author

still requires manual engagement of the runner to perform any testing of a PR from an external source

We will be using fixed source ( files on the Runner) and auto-mode running. There will be no manual engagement.

@benlee0423
Copy link
Author

Updated PR description for clarification.

@arpita0911patel
Copy link
Member

Merging it to test it works for the forked PRs and then if any issues we can fix it after that.

@arpita0911patel arpita0911patel merged commit bcc9dcf into main Feb 14, 2024
15 of 16 checks passed
@benlee0423 benlee0423 deleted the x86-build branch February 16, 2024 03:32
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