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

Remove debug print #3389

Merged
merged 1 commit into from
Apr 23, 2024
Merged

Conversation

djmitche
Copy link
Collaborator

Fixes #3386.

I'm surprised this didn't cause test failures!

@djmitche djmitche requested a review from ryneeverett April 22, 2024 19:45
@djmitche
Copy link
Collaborator Author

I think we should release 3.0.2 with this fix -- thoughts?

@Bckempa
Copy link

Bckempa commented Apr 22, 2024

Thank you for the quick fix!

I think we should release 3.0.2 with this fix -- thoughts?

I've got a minor nit about how news works (that is, unless you know about news all you'll always be randomly bugged to (re-)read the news. Do you want to take a look at that for a 3.0.2 or is that not worth holding this back?

@djmitche
Copy link
Collaborator Author

Please file that as an issue - I agree but it was clearly designed that way so I'm not sure we should change it.

I'd like to ship 3.0.2 as a bugfix for this serious bug.

@ryneeverett
Copy link
Collaborator

I'm surprised this didn't cause test failures!

I suspect this is because the cli tests make heavy use of assertIn and so allow unlimited "extra" output. The most basic tests for each command ought to more strictly validate the output than they do.

@djmitche djmitche merged commit 9b35ab3 into GothenburgBitFactory:develop Apr 23, 2024
21 checks passed
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.

Version prepended to task lists
3 participants