Uh oh!
There was an error while loading. Please reload this page.
GH-137623: Use an AC decorator for docstring line length enforcement - #137690
Conversation
fbb070c to
37dbdf7Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
AA-Turner
commented
Aug 18, 2025
I'd prefer to merge this, I think it's a cleaner solution than the current. @serhiy-storchaka do you have any objections? A |
serhiy-storchaka
commented
Aug 18, 2025
LGTM. 👍 |
AA-Turner
commented
Aug 18, 2025
Google translate doesn't have any suggestions here! |
AA-Turner
commented
Aug 18, 2025
Thank you! |
Uh oh!
There was an error while loading. Please reload this page.
Oh, I would have suggested |
AA-Turner
commented
Aug 18, 2025
Sorry, I wanted to minimise review spam so kept it in draft. Happy to change it if you'd like? My rationale for "permit" instead of "allow" was to signal that this is an exception to the rule -- we don't want to see the number of exceptions go up here. "allow" could be seen as a supported way to bypass the checks, which we'd prefer to avoid. I realise that this is a very slight semantic difference, though! |
picnixz
commented
Aug 18, 2025
Oh, if that's your rationale, I'm ok with it though in this case I would have had some "ignore_maxline_length" instead or "disable_docstring_length_check" but I'm fine the |
skirpichev
commented
Aug 19, 2025
ДПЕЬ is LGTM in Russian (or Ukrainian, I think that part of keyboard has no difference). Shame on Google! |
After thinking about it, @serhiy-storchaka's suggestion to use a decorator seems a better approach. It keeps the information localised to where the problem is, rather than hiding it in
libclinic, which most contributors shouldn't need to worry about.Opening as draft to avoid review-request spam until ready for merge.
A