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.

Sign up for GitHub

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

Jump to bottom

Revert "ovirt / rhv: drop swap partition" #3474

Merged
merged 2 commits into from Jun 30, 2021
Merged

Revert "ovirt / rhv: drop swap partition" #3474

merged 2 commits into from Jun 30, 2021

Conversation

lveyde
Copy link
Contributor

@lveyde lveyde commented Jun 28, 2021

This reverts commit b894cd1.

It was decided that it's better to have swap partition after all.

RHBZ#1976005

Signed-off-by: Lev Veyde lveyde@redhat.com

@lveyde
Revert "ovirt / rhv: drop swap partition"
5dae874 
This reverts commit b894cd1.

It was decided that it's better to have swap partition after all.

Signed-off-by: Lev Veyde <lveyde@redhat.com>
@lveyde
Copy link
Contributor Author

lveyde commented Jun 28, 2021

@sandrobonazzola

VladimirSlavik
VladimirSlavik previously approved these changes Jun 28, 2021
View reviewed changes
@VladimirSlavik VladimirSlavik added the master Please, use the `f39` label instead. label Jun 28, 2021
@VladimirSlavik
Copy link
Contributor

VladimirSlavik commented Jun 28, 2021 •
edited

Looks like new pylint gives us new exciting errors. the code merged earlier needs new dependencies, but test containers are built daily. Sorry. Otherwise tests look good. you need to revert also some change to tests, I think? IIRC you had two commits, but I didn't look up the original PR.

@VladimirSlavik VladimirSlavik dismissed their stale review June 28, 2021 14:56

Test must pass, I thought they were stuck on pylint but that's not the case.

@lveyde
Revert "Fix unit test for previous commit"
e9a5832 
This reverts commit fd81732.

Signed-off-by: Lev Veyde <lveyde@redhat.com>
@lveyde
Copy link
Contributor Author

lveyde commented Jun 28, 2021

Looks like new pylint gives us new exciting errors. the code merged earlier needs new dependencies, but test containers are built daily. Sorry. Otherwise tests look good. you need to revert also some change to tests, I think? IIRC you had two commits, but I didn't look up the original PR.

Added a revert for the original unit test as well.

@VladimirSlavik
Copy link
Contributor

Thanks, that looks good. The "tests needs new dependency" issue is still there, but that should be ok tomorrow.

poncovka
poncovka approved these changes Jun 29, 2021
View reviewed changes
Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

@poncovka
Copy link
Contributor

/kickstart-test --testtype smoke

@VladimirSlavik
Copy link
Contributor

/packit test

VladimirSlavik
VladimirSlavik approved these changes Jun 29, 2021
View reviewed changes
Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

@poncovka poncovka self-assigned this Jun 30, 2021
@poncovka poncovka merged commit 7891542 into rhinstaller:master Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sandrobonazzola sandrobonazzola sandrobonazzola approved these changes

@poncovka poncovka poncovka approved these changes

@VladimirSlavik VladimirSlavik VladimirSlavik approved these changes

Assignees

@poncovka poncovka

Labels
master Please, use the `f39` label instead.
Milestone
No milestone
4 participants
@lveyde @VladimirSlavik @poncovka @sandrobonazzola