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: don't fetch test code from Yi's repo #134

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

henrywang
Copy link

@henrywang henrywang commented Nov 14, 2023

This PR remove some un-used test code for greenboot-rs test. The greenboot-rs integration test code will be moved into greenboot-rs branch by PR #133 instead of fetch code from Yi's repo.

test: remove un-used code for cleanup
@yih-redhat
Copy link
Collaborator

yih-redhat commented Nov 14, 2023

The original plan is to add the test scripts in branch greenboot-rs, and then Sayan can make a rebase in his pr #100 to trigger the test.
After discussed with Sayan, he mentioned rebase could cause some problems now, so I put the test scripts in my repo, and plan to move them into greenboot-rs branch after Sayan merged his pr #100

@henrywang
Copy link
Author

henrywang commented Nov 14, 2023

They are all about test code, and not related with greenboot code. So the rebase should work because the rebase is just to add one more tests folder. Or you can add test code into PR #100. That can avoid issue. So let's land this PR first.

@henrywang
Copy link
Author

@say-paul Could you please review this PR? Thanks!

@say-paul
Copy link
Member

say-paul commented Dec 19, 2023

I suggested against it earlier as the tests were more inline with the current green-boot, which would fail the new -rs branch. Since its now inline with the greenboot-rs, its safe to merge.

@say-paul
Copy link
Member

say-paul commented Dec 19, 2023

greenboot-rs built process is itself broken, and needs some update, merging a tests on that branch wont give us any advantage. so we can keep the pr #133 open until #100 is merged.

But this PR makes sense and look good to me.

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