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

Support docformatter 1.6.0+ #18790

Merged
merged 1 commit into from
Apr 22, 2023
Merged

Conversation

benjyw
Copy link
Contributor

@benjyw benjyw commented Apr 22, 2023

Docformatter 1.6.0 introduced a "feature" that is actually a nuisance:
it returns exit code 3 when modifying files in-place.

We don't actually upgrade the default version in this PR, because the new
version of docformatter has logic changes that wreak havoc on our docstrings.

But I have tested this change on both old and new versions of Docformatter.

@benjyw benjyw added the category:internal CI, fixes for not-yet-released features, etc. label Apr 22, 2023
@benjyw benjyw requested a review from thejcannon April 22, 2023 17:07
@benjyw
Copy link
Contributor Author

benjyw commented Apr 22, 2023

Fixes #18706

@benjyw benjyw requested a review from Eric-Arellano April 22, 2023 17:08
# (All versions return 3 in check mode if any files would have changed, but that is
# not an issue here).
if result.exit_code not in [0, 3]:
# TODO(#12725):It would be more straightforward to force the exception with:
Copy link
Member

Choose a reason for hiding this comment

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

I don't get the reference to #12725

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I cargo-culted that from other files that mentioned the inability to convert FallibleProcessResult to ProcessResult.

Yeah, I'm not 100% sure why that is the referenced PR.

Copy link
Member

Choose a reason for hiding this comment

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

also, I would expect the TODO to reference an open issue.. ;)

@benjyw benjyw merged commit 9b412f0 into pantsbuild:main Apr 22, 2023
@benjyw benjyw deleted the support_new_docformatter branch April 22, 2023 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:internal CI, fixes for not-yet-released features, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants