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

e2e rework: test_vm_actions.py #940

Merged
merged 4 commits into from
Sep 25, 2023
Merged

Conversation

lanfon72
Copy link
Member

@lanfon72 lanfon72 commented Sep 18, 2023

Changes

@lanfon72 lanfon72 force-pushed the e2e-rework branch 2 times, most recently from 971b392 to 976e8d4 Compare September 18, 2023 18:17
@lanfon72 lanfon72 marked this pull request as ready for review September 18, 2023 18:18
@lanfon72 lanfon72 requested a review from a team September 18, 2023 18:19
@lanfon72 lanfon72 mentioned this pull request Sep 18, 2023
Copy link
Contributor

@albinsun albinsun left a comment

Choose a reason for hiding this comment

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

Suggest to rename _cb to something more explicit one like default_cb which may ease further code tracing and maintenance.

harvester_e2e_tests/fixtures/virtualmachines.py Outdated Show resolved Hide resolved
harvester_e2e_tests/fixtures/virtualmachines.py Outdated Show resolved Hide resolved
harvester_e2e_tests/fixtures/virtualmachines.py Outdated Show resolved Hide resolved
harvester_e2e_tests/fixtures/virtualmachines.py Outdated Show resolved Hide resolved
@albinsun albinsun requested a review from a team September 20, 2023 02:10
@lanfon72 lanfon72 force-pushed the e2e-rework branch 2 times, most recently from 4fbc1cb to 6cca196 Compare September 20, 2023 05:18
@lanfon72
Copy link
Member Author

Tested on harvester-install-and-test#886

@lanfon72 lanfon72 requested a review from albinsun September 20, 2023 13:59
Copy link
Contributor

@albinsun albinsun left a comment

Choose a reason for hiding this comment

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

LGTM, with some minor comments.

@albinsun albinsun requested a review from a team September 22, 2023 10:21
@khushboo-rancher khushboo-rancher merged commit c349b06 into harvester:main Sep 25, 2023
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