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

simplify TestValues enum creation and usage #4220

Merged
merged 1 commit into from
Jun 30, 2022

Conversation

ethanfurman
Copy link
Contributor

Enhance #4216.

@VladimirSlavik
Copy link
Contributor

Thank you! That looks a lot better, but the tests disagree, I'm afraid :-(

@ethanfurman
Copy link
Contributor Author

Wow, it's actually embarassing how many times I forget to return member at the end of __new__.

It should work much better now. :-)

@VladimirSlavik VladimirSlavik added the f37 Fedora 37 label Jun 29, 2022
@VladimirSlavik
Copy link
Contributor

Thank you, that looks great! Two things still:

  • Do you think it would be better to explicitly have None as the value for the "broken" variants?
  • Can you please squash this so that it's one commit? I can do that myself, if you want.

@ethanfurman
Copy link
Contributor Author

Specifying None on the three broken variants is a matter of taste. I wouldn't usually do it myself, but I have no experience with your code base to know which is more consistent with its style.

As for squashing, feel free to do so as I have no idea how -- my git skills are fairly rudimentary.

@VladimirSlavik
Copy link
Contributor

So, I squashed it and made the None explicit, so it's easier to get for the next generations. Your local branch is likely "broken" for you now, unless you delete it and pull from your fork.

@VladimirSlavik
Copy link
Contributor

/kickstart-test --testtype smoke

@VladimirSlavik VladimirSlavik self-assigned this Jun 30, 2022
@VladimirSlavik VladimirSlavik merged commit ea34277 into rhinstaller:master Jun 30, 2022
@ethanfurman ethanfurman deleted the simple-enum branch June 30, 2022 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f37 Fedora 37
2 participants