You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Jarek Potiuk <ja...@potiuk.com> on 2024/03/20 04:07:33 UTC
[DISCUSS] Applying D105 rule for our codebase ("undocumented magic methods") ?
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.
Re: [DISCUSS] Applying D105 rule for our codebase ("undocumented magic methods") ?
Posted by Jarek Potiuk <ja...@potiuk.com>.
PR to remove it https://github.com/apache/airflow/pull/38452
On Mon, Mar 25, 2024 at 10:14 AM Jarek Potiuk <ja...@potiuk.com> wrote:
> Ok. Seems we have a consensus :)
>
> On Mon, Mar 25, 2024 at 7:13 AM Wei Lee <we...@gmail.com> 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 <am...@gmail.com>
>> 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
>> <on...@amazon.com.invalid>
>> > 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 <vi...@apache.org>
>> >> Sent: Wednesday, March 20, 2024 5:51:43 AM
>> >> To: dev@airflow.apache.org
>> >> 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 <
>> andrey.anshin@taragol.is>
>> >> 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 <pankaj.koti@astronomer.io
>> >> .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 <
>> >> aritrabasu1999@gmail.com>
>> >>>>> 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 <ja...@potiuk.com>
>> >> 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: dev-unsubscribe@airflow.apache.org
>> >> For additional commands, e-mail: dev-help@airflow.apache.org
>> >>
>> >>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@airflow.apache.org
>> For additional commands, e-mail: dev-help@airflow.apache.org
>>
>>
Re: [DISCUSS] Applying D105 rule for our codebase ("undocumented magic methods") ?
Posted by Jarek Potiuk <ja...@potiuk.com>.
Ok. Seems we have a consensus :)
On Mon, Mar 25, 2024 at 7:13 AM Wei Lee <we...@gmail.com> 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 <am...@gmail.com>
> 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
> <on...@amazon.com.invalid>
> > 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 <vi...@apache.org>
> >> Sent: Wednesday, March 20, 2024 5:51:43 AM
> >> To: dev@airflow.apache.org
> >> 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 <andrey.anshin@taragol.is
> >
> >> 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 <pankaj.koti@astronomer.io
> >> .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 <
> >> aritrabasu1999@gmail.com>
> >>>>> 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 <ja...@potiuk.com>
> >> 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: dev-unsubscribe@airflow.apache.org
> >> For additional commands, e-mail: dev-help@airflow.apache.org
> >>
> >>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@airflow.apache.org
> For additional commands, e-mail: dev-help@airflow.apache.org
>
>
Re: [DISCUSS] Applying D105 rule for our codebase ("undocumented magic methods") ?
Posted by Wei Lee <we...@gmail.com>.
+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 <am...@gmail.com> 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 <on...@amazon.com.invalid>
> 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 <vi...@apache.org>
>> Sent: Wednesday, March 20, 2024 5:51:43 AM
>> To: dev@airflow.apache.org
>> 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 <an...@taragol.is>
>> 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 <pankaj.koti@astronomer.io
>> .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 <
>> aritrabasu1999@gmail.com>
>>>>> 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 <ja...@potiuk.com>
>> 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: dev-unsubscribe@airflow.apache.org
>> For additional commands, e-mail: dev-help@airflow.apache.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@airflow.apache.org
For additional commands, e-mail: dev-help@airflow.apache.org
Re: [DISCUSS] Applying D105 rule for our codebase ("undocumented magic methods") ?
Posted by Amogh Desai <am...@gmail.com>.
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 <on...@amazon.com.invalid>
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 <vi...@apache.org>
> Sent: Wednesday, March 20, 2024 5:51:43 AM
> To: dev@airflow.apache.org
> 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 <an...@taragol.is>
> 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 <pankaj.koti@astronomer.io
> .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 <
> aritrabasu1999@gmail.com>
> > >> 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 <ja...@potiuk.com>
> 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: dev-unsubscribe@airflow.apache.org
> For additional commands, e-mail: dev-help@airflow.apache.org
>
>
Re: [DISCUSS] Applying D105 rule for our codebase ("undocumented magic methods") ?
Posted by "Oliveira, Niko" <on...@amazon.com.INVALID>.
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 <vi...@apache.org>
Sent: Wednesday, March 20, 2024 5:51:43 AM
To: dev@airflow.apache.org
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 <an...@taragol.is> 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 <pa...@astronomer.io.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 <ar...@gmail.com>
> >> 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 <ja...@potiuk.com> 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: dev-unsubscribe@airflow.apache.org
For additional commands, e-mail: dev-help@airflow.apache.org
Re: [DISCUSS] Applying D105 rule for our codebase ("undocumented magic methods") ?
Posted by Vincent Beck <vi...@apache.org>.
+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 <an...@taragol.is> 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 <pa...@astronomer.io.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 <ar...@gmail.com>
> >> 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 <ja...@potiuk.com> 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: dev-unsubscribe@airflow.apache.org
For additional commands, e-mail: dev-help@airflow.apache.org
Re: [DISCUSS] Applying D105 rule for our codebase ("undocumented magic methods") ?
Posted by Ash Berlin-Taylor <as...@apache.org>.
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 <an...@taragol.is> 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 <pa...@astronomer.io.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 <ar...@gmail.com>
>> 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 <ja...@potiuk.com> 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.
>> > >
>> >
>>
Re: [DISCUSS] Applying D105 rule for our codebase ("undocumented magic methods") ?
Posted by Andrey Anshin <an...@taragol.is>.
±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 <pa...@astronomer.io.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 <ar...@gmail.com>
> 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 <ja...@potiuk.com> 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.
> > >
> >
>
Re: [DISCUSS] Applying D105 rule for our codebase ("undocumented magic methods") ?
Posted by Pankaj Koti <pa...@astronomer.io.INVALID>.
+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 <ar...@gmail.com>
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 <ja...@potiuk.com> 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.
> >
>
Re: [DISCUSS] Applying D105 rule for our codebase ("undocumented magic methods") ?
Posted by Aritra Basu <ar...@gmail.com>.
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 <ja...@potiuk.com> 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.
>