GH-137623: Use an AC decorator for docstring line length enforcement #137690
Conversation
AA-Turner
force-pushed
the
clinic/overlong-docstrings-deco
branch
from
fbb070c to
37dbdf7
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
I'd prefer to merge this, I think it's a cleaner solution than the current. @serhiy-storchaka do you have any objections? A |
|
LGTM. 👍 |
Google translate doesn't have any suggestions here! |
|
Thank you! |
AA-Turner
requested review from
pablogsal,
ambv,
1st1,
ericsnowcurrently,
erlend-aasland,
corona10,
FFY00,
rhettinger,
a team,
gpshead,
berkerpeksag,
pganssle and
abalkin
as code owners
AA-Turner
requested review from
picnixz,
markshannon and
methane
as code owners
AA-Turner
removed request for
a team,
berkerpeksag,
ambv,
gpshead,
methane,
1st1,
abalkin,
ericsnowcurrently,
pganssle,
rhettinger,
corona10,
markshannon,
picnixz,
pablogsal,
FFY00,
erlend-aasland and
kumaraditya303
|
Oh, I would have suggested |
|
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! |
|
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 |
ДПЕЬ 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