±0 from my side Maybe we have to review all current methods which do not follow this rule to find a really useful meaning, and do not enforce (disable it). So for avoid unnecessary changes we might close https://github.com/apache/airflow/issues/37523 and remove/mark completed into the https://github.com/apache/airflow/issues/10742
On Wed, 20 Mar 2024 at 10:41, Pankaj Koti <[email protected]> wrote: > +1 to what Aritra is saying. > > > Best regards, > > *Pankaj Koti* > Senior Software Engineer (Airflow OSS Engineering team) > Location: Pune, Maharashtra, India > Timezone: Indian Standard Time (IST) > Phone: +91 9730079985 > > > On Wed, Mar 20, 2024 at 12:05 PM Aritra Basu <[email protected]> > wrote: > > > I'm in general not a huge fan of documenting for the sake of documenting, > > so I'd be in agreement of not enforcing it via code but rather be > enforced > > by the reviewers in cases they believe certain methods need documenting. > > > > -- > > Regards, > > Aritra Basu > > > > On Wed, Mar 20, 2024, 9:39 AM Jarek Potiuk <[email protected]> wrote: > > > > > Hey here, > > > > > > I wanted to quickly poll what people think about applying the > > > https://docs.astral.sh/ruff/rules/undocumented-magic-method/ rule in > our > > > codebase. There are many uncontroversial rules - but that one is > somewhat > > > more controversial than others. > > > > > > See > https://github.com/apache/airflow/pull/37602#issuecomment-2001951402 > > > and > > > > > > https://github.com/apache/airflow/pull/38277#pullrequestreview-1945745542 > > > for example > > > > > > I think that even in the ruff example, but also in many cases requiring > > to > > > document the methods will lead to rather useless documentation: > > > > > > class Cat(Animal): > > > def __str__(self) -> str: > > > """Return a string representation of the cat.""" > > > return f"Cat: {self.name}" > > > > > > There is IMHO very little value in having such documentation. It might > be > > > useful in some cases where we have a really good reason to add such a > > magic > > > method and it is important to document it, but in many cases - the > > > documentation will be just documenting what the magic method already > > > explains well (like the case above). > > > > > > This actually reminds me the early days of java documentation where > > javadoc > > > looks more or less like this: > > > > > > "Paints the object" > > > func paint() > > > > > > "Repaints the object" > > > fund repaint() > > > > > > However - maybe I am wrong :). Maybe it's worth documenting those > methods > > > in bulk, even if in many cases it will not bring much value? > > > > > > WDYT ? Should we mandate documenting it - or leave up to the author to > > > document it in case it feels like it is needed? > > > > > > J. > > > > > >
