PR to remove it https://github.com/apache/airflow/pull/38452
On Mon, Mar 25, 2024 at 10:14 AM Jarek Potiuk <[email protected]> wrote: > Ok. Seems we have a consensus :) > > On Mon, Mar 25, 2024 at 7:13 AM Wei Lee <[email protected]> wrote: > >> +1 for removing this rule. If docstring is needed for some cases, we can >> still do that or add comments to PRs. >> >> Best, >> Wei >> >> > On Mar 25, 2024, at 1:31 PM, Amogh Desai <[email protected]> >> wrote: >> > >> > After reading the emails in this thread, I too think that this rule is >> not >> > generally very useful, but we should have >> > it for cases which are special (where we are overriding some magic >> method >> > to do something different or more >> > than its reserved meaning) >> > >> > +1 for removal of the rule, with special cases being handled separately. >> > >> > Thanks & Best Regards, >> > Amogh Desai >> > >> > On Wed, Mar 20, 2024 at 11:46 PM Oliveira, Niko >> <[email protected]> >> > wrote: >> > >> >> I'm -1 to enabling D105 >> >> >> >> >> >> I don't think it will lead to helpful documentation. I think for the >> rare >> >> cases it is required it can left up to the developer or caught in PR >> review. >> >> >> >> Cheers, >> >> Niko >> >> >> >> ________________________________ >> >> From: Vincent Beck <[email protected]> >> >> Sent: Wednesday, March 20, 2024 5:51:43 AM >> >> To: [email protected] >> >> Subject: RE: [EXTERNAL] [COURRIEL EXTERNE] [DISCUSS] Applying D105 rule >> >> for our codebase ("undocumented magic methods") ? >> >> >> >> CAUTION: This email originated from outside of the organization. Do not >> >> click links or open attachments unless you can confirm the sender and >> know >> >> the content is safe. >> >> >> >> >> >> >> >> AVERTISSEMENT: Ce courrier électronique provient d’un expéditeur >> externe. >> >> Ne cliquez sur aucun lien et n’ouvrez aucune pièce jointe si vous ne >> pouvez >> >> pas confirmer l’identité de l’expéditeur et si vous n’êtes pas certain >> que >> >> le contenu ne présente aucun risque. >> >> >> >> >> >> >> >> +1 for not enforcing as well. Let's leave to maintainers the >> flexibility >> >> to chose whether a given method should be documented. >> >> >> >> On 2024/03/20 08:28:51 Ash Berlin-Taylor wrote: >> >>> I'm for not enforcing this rule - as others have said its very >> unlikely >> >> to result in more useful docs for developers or end users. >> >>> >> >>> -asg >> >>> >> >>> On 20 March 2024 08:12:40 GMT, Andrey Anshin < >> [email protected]> >> >> wrote: >> >>>> ±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] >> >> .invalid> >> >>>> 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. >> >>>>>>> >> >>>>>> >> >>>>> >> >>> >> >> >> >> --------------------------------------------------------------------- >> >> To unsubscribe, e-mail: [email protected] >> >> For additional commands, e-mail: [email protected] >> >> >> >> >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> >>
