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

PEP 7: Add note about greppable exception messages #2605

Merged
merged 2 commits into from May 13, 2022

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented May 13, 2022

Resolves #2604

Copy link
Member

@gvanrossum gvanrossum left a comment

Thank you!

Copy link
Member

@ericvsmith ericvsmith left a comment

Looks good!

@erlend-aasland erlend-aasland requested a review from CAM-Gerlach May 13, 2022
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

One minor wording suggestion to make the phrasing a little easier to follow, otherwise LGTM from an editorial perspective. Thanks @erlend-aasland !

pep-0007.txt Outdated Show resolved Hide resolved
@gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented May 13, 2022

I wonder if we should add the same thing to PEP 8?

Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>
@erlend-aasland erlend-aasland changed the title PEP 7: Add note about greppable exception messages PEP 7/8: Add note about greppable exception messages May 13, 2022
@erlend-aasland
Copy link
Contributor Author

@erlend-aasland erlend-aasland commented May 13, 2022

I wonder if we should add the same thing to PEP 8?

Yes. I'll create a separate PR for PEP 8.

@erlend-aasland erlend-aasland changed the title PEP 7/8: Add note about greppable exception messages PEP 7: Add note about greppable exception messages May 13, 2022
@erlend-aasland erlend-aasland merged commit c787739 into python:main May 13, 2022
4 checks passed
@erlend-aasland erlend-aasland deleted the pep7/long-lines branch May 13, 2022
@warsaw
Copy link
Member

@warsaw warsaw commented May 16, 2022

As I said over here, I've changed my mind on whether this is a useful recommendation to add to PEP 7 (and 8).

@CAM-Gerlach
Copy link
Member

@CAM-Gerlach CAM-Gerlach commented May 16, 2022

The PEP 8 change seems potentially far more impactful than the PEP 7 one, as the latter will mainly only impact the stdlib, while the former is treated as the de-facto standard for formatting all Python code, which theoretically could affect on the order of >=billions of lines of code versus less than 500 000. So I could see a justification to apply this only to PEP 7 on those grounds, However, on the other hand, it might be inconsistent to only do one and not the other, unless there is another plausible reason it should apply to the C code but not the Python code.

@erlend-aasland
Copy link
Contributor Author

@erlend-aasland erlend-aasland commented May 16, 2022

I agree, CAM, that it could make sense to keep the PEP 7 change. Speaking from my own experience, I rarely need to grep for exception messages in Python files; I can find those location directly from a traceback. If I grep for an exception message, it is almost always because I want to find the C code that raised it.

erlend-aasland added a commit to erlend-aasland/peps that referenced this issue May 16, 2022
)"

Based on post-merge feedback, it was decided to revert this change.

See the original PR (pythonGH-2605), the proposed PR for PEP 8 (pythonGH-2606), and
the issue (pythonGH-2604) for the discussions.

This reverts commit c787739.
erlend-aasland added a commit that referenced this issue May 17, 2022
…2608)

Based on post-merge feedback, it was decided to revert this change.

See the original PR (GH-2605), the proposed PR for PEP 8 (GH-2606), and
the issue (GH-2604) for the discussions.

This reverts commit c787739.
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.

5 participants