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

Make command line deprecation warnings visible with test profile #3665

Merged

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Dec 13, 2019

The deprecated_command decorator was only printing the warning if the
profile was not a test profile. With verdi devel tests being
deprecated, typically run with a test profile, the warning was being
swallowed.

@sphuber sphuber requested a review from ltalirz December 13, 2019 13:44
ltalirz
ltalirz previously approved these changes Dec 13, 2019
Copy link
Member

@ltalirz ltalirz left a comment

Choose a reason for hiding this comment

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

@sphuber looks good to me
didn't check why the tests are failing (is this a pre-commit issue?)

The `deprecated_command` decorator was only printing the warning if the
profile was not a test profile. With `verdi devel tests` being
deprecated, typically run with a test profile, the warning was being
swallowed.

This change gave problems with a test of `verdi comment show` that
expected no output for no input, but now of course failed due to the
output of the deprecation. Since it is a deprecated command anyway I
simply remove the test.
@sphuber sphuber force-pushed the fix_verdi_devel_tests_deprecated branch from 0fd104c to eac4d84 Compare December 13, 2019 14:26
@sphuber sphuber requested a review from ltalirz December 13, 2019 14:36
@sphuber
Copy link
Contributor Author

sphuber commented Dec 13, 2019

@ltalirz I had to update it a bit because the tests failed

@giovannipizzi giovannipizzi merged commit 7895662 into aiidateam:develop Dec 13, 2019
@giovannipizzi giovannipizzi deleted the fix_verdi_devel_tests_deprecated branch December 13, 2019 15:47
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