Yup agreed with both: going ahead with new rules and excluding D102 and D103
On Sun, 14 Jul 2024 at 22:17, Jarek Potiuk <ja...@potiuk.com> wrote: > On Sun, Jul 14, 2024 at 6:34 PM Ash Berlin-Taylor <a...@apache.org> wrote: > > > I agree with everything you've identified except these two: > > > > D102 and D103 aren't worth it for us - there are lots of "public" > > functions from classes but we don't need rendered docs for them unless > they > > are part of the "public python API". High effort to get good doc strings > > for low return on my view. > > > > Maybe once we have a more defined public python API (i.e. -72) then we > > could turn those two on for those files. > > > > > Fully agree with Ash here. > > > > > -ash > > > > On 12 July 2024 21:24:24 BST, "Ferruzzi, Dennis" > > <ferru...@amazon.com.INVALID> wrote: > > >Hey folks, > > > > > >There have been a number of projects over the years to enable more and > > more style rules in our linters. They serve as a great first project and > > help to standardize code which makes it easier for Future Us to jump into > > sections of the code we otherwise don't usually work in and for new > > contributors to ramp up. The current batch are winding down and I figure > > it's time to get a temperature check for another batch. > > > > > >This is almost certainly going to open a can of worms since python is > > inherently a non-prescriptive language and everyone will have their own > > preferences but hopefully we can pick a couple that we can agree wouldn't > > hurt to enable. > > > > > >The easy part of my proposal is that each ignored rule should have a > > one-liner comment EITHER marking it as unwanted (and why) or as TODO. > I'll > > consider that part to fall under lazy consensus and can make that change > at > > the end of this discussion. > > > > > >The trickier part will be deciding which ones are which. Below is a > list > > of the ones which are currently disabled, a link to the detailed > > explanation of the rule, a one-liner explaining the rule for those who > > don't want to click, how many violations exist in the codebase right now > > (for a sense of scale), and my personal suggestion on each one. > > > > > > > > > > > >"D100<https://docs.astral.sh/ruff/rules/undocumented-public-module/>", > # > > Unwanted: Docstring at the top of every file. > > >"D102<https://docs.astral.sh/ruff/rules/undocumented-public-method/>", > # > > TODO: Docstring in every public method; currently 2473 violations > > >"D103<https://docs.astral.sh/ruff/rules/undocumented-public-function/ > >", > > # TODO: Docstring in every public function; currently 318 violations > > >"D104<https://docs.astral.sh/ruff/rules/undocumented-public-package/>", > > # Unwanted: Docstring in every `__init__.py` file. > > >"D105<https://docs.astral.sh/ruff/rules/undocumented-magic-method/>", # > > Unwanted: Docstring in every magic method, See > > https://lists.apache.org/thread/8jbg1dd2lr2cfydtqbjxsd6pb6q2wkc3 > > >"D107<https://docs.astral.sh/ruff/rules/undocumented-public-init/>", # > > Unwanted: Docstring in every constructor is unnecessary if the class has > a > > docstring. > > >"D203<https://docs.astral.sh/ruff/rules/one-blank-line-before-class/>", > > # Unwanted: Conflicts with D211. Both can not be enabled. > > >"D212<https://docs.astral.sh/ruff/rules/multi-line-summary-first-line/ > >", > > # Unwanted: Conflicts with D213. Both can not be enabled. > > >"D214<https://docs.astral.sh/ruff/rules/section-not-over-indented/>", # > > TODO: Consistent multi-line docstring indentation; currently 0 violations > > >"D215< > > https://docs.astral.sh/ruff/rules/section-underline-not-over-indented/ > >", > > # TODO: Enforce number of underscores if you use underlines in a > multi-line > > docstring; currently 0 violations > > >"E731<https://docs.astral.sh/ruff/rules/lambda-assignment/>", # MAYBE?: > > Use a method instead of a lambda when declaring a variable; currently 3 > > violations > > >"TCH003< > > https://docs.astral.sh/ruff/rules/typing-only-standard-library-import/ > >", > > # MAYBE?: stdlib imports which are only used for type checking go into > > TYPE_CHECKING block; currently 38 violations > > >"PT004< > > > https://docs.astral.sh/ruff/rules/pytest-missing-fixture-name-underscore/ > >", > > # TODO: Fixture does not return anything, add leading underscore; > currently > > 311 violations > > >"PT005< > > > https://docs.astral.sh/ruff/rules/pytest-incorrect-fixture-name-underscore/ > >", > > # Unwanted: Fixture returns a value, remove leading underscore; > currently 1 > > violation > > >"PT006< > > https://docs.astral.sh/ruff/rules/pytest-parametrize-names-wrong-type/ > >", > > # TODO: @pytest.mark.parametrize names should be a tuple; currently 1215 > > violations > > >"PT007< > > https://docs.astral.sh/ruff/rules/pytest-parametrize-values-wrong-type/ > >", > > # TODO: @pytest.mark.parametrize values should be a tuple; currently 391 > > violations > > >"PT011<https://docs.astral.sh/ruff/rules/pytest-raises-too-broad/>", # > > TODO: pytest.raises() is too broad, set the match parameter; currently > 153 > > violations > > >"PT019< > > https://docs.astral.sh/ruff/rules/pytest-fixture-param-without-value/>", > > # MAYBE?: Use @pytest.mark.usefixtures instead of passing a fixture as a > > parameter; currently 268 violations > > > > > > > > >What do we think? Some seem pretty obvious like D214 which is disabled > > but currently has no violations so we may as well enable it. Some I > don't > > have strong feelings about but think we may as well enable them. And > > others like PT019 I have honestly no clue. I think overall I'd lean > > towards "enable unless we have a good reason not to" as a general > guideline? > > > > > > > > > > > > - ferruzzi > > >
