1. Topics
  2. Adding doc requirement to commit guidelines

Adding doc requirement to commit guidelines

rschlussel@...
 

Hi everyone,

 

It’s come up a few times recently where new functions have been added without documentation, which makes them not very usable.  Since it’s much easier to ensure that documentation gets written if it’s done when the feature is added, I’d like to propose an update to our commit guidelines to say that new functions, language features, etc. need to have documentation included as part of the PR, and we don’t merge without that documentation.

 

What do others think?

 

Rebecca
Ariel Weisberg
 

Hi,

Agree. It's an implicit requirement IMO, but I see people (sometimes me) forgetting it at both contribution and review time.

Ariel

On Thu, Mar 4, 2021, at 12:14 PM, rschlussel via lists.prestodb.io wrote:

Hi everyone,

 

It’s come up a few times recently where new functions have been added without documentation, which makes them not very usable.  Since it’s much easier to ensure that documentation gets written if it’s done when the feature is added, I’d like to propose an update to our commit guidelines to say that new functions, language features, etc. need to have documentation included as part of the PR, and we don’t merge without that documentation.

 

What do others think?

 

Rebecca
Maria Basmanova
 

Makes sense to me too.

 

-Masha

 

From: <presto-tsc@...> on behalf of Ariel Weisberg <ariel@...>
Date: Thursday, March 4, 2021 at 12:18 PM
To: "presto-tsc@..." <presto-tsc@...>
Subject: Re: [presto-tsc] Adding doc requirement to commit guidelines

 

Hi,

 

Agree. It's an implicit requirement IMO, but I see people (sometimes me) forgetting it at both contribution and review time.

 

Ariel

 

On Thu, Mar 4, 2021, at 12:14 PM, rschlussel via lists.prestodb.io wrote:

Hi everyone,

 

It’s come up a few times recently where new functions have been added without documentation, which makes them not very usable.  Since it’s much easier to ensure that documentation gets written if it’s done when the feature is added, I’d like to propose an update to our commit guidelines to say that new functions, language features, etc. need to have documentation included as part of the PR, and we don’t merge without that documentation.

 

What do others think?

 

Rebecca

 
zluo@...
 

+1

On Thu, Mar 4, 2021 at 10:44 AM Maria Basmanova via lists.prestodb.io <mbasmanova=fb.com@...> wrote:

Makes sense to me too.

 

-Masha

 

From: <presto-tsc@...> on behalf of Ariel Weisberg <ariel@...>
Date: Thursday, March 4, 2021 at 12:18 PM
To: "presto-tsc@..." <presto-tsc@...>
Subject: Re: [presto-tsc] Adding doc requirement to commit guidelines

 

Hi,

 

Agree. It's an implicit requirement IMO, but I see people (sometimes me) forgetting it at both contribution and review time.

 

Ariel

 

On Thu, Mar 4, 2021, at 12:14 PM, rschlussel via lists.prestodb.io wrote:

Hi everyone,

 

It’s come up a few times recently where new functions have been added without documentation, which makes them not very usable.  Since it’s much easier to ensure that documentation gets written if it’s done when the feature is added, I’d like to propose an update to our commit guidelines to say that new functions, language features, etc. need to have documentation included as part of the PR, and we don’t merge without that documentation.

 

What do others think?

 

Rebecca

 
Bin Fan
 

Agree.

If the original PR in src code is already large, perhaps we shall allow another separate doc PR linked in the first PR?

- Bin

On Thu, Mar 4, 2021 at 9:14 AM rschlussel via lists.prestodb.io <rschlussel=fb.com@...> wrote:

Hi everyone,

 

It’s come up a few times recently where new functions have been added without documentation, which makes them not very usable.  Since it’s much easier to ensure that documentation gets written if it’s done when the feature is added, I’d like to propose an update to our commit guidelines to say that new functions, language features, etc. need to have documentation included as part of the PR, and we don’t merge without that documentation.

 

What do others think?

 

Rebecca



--

rschlussel@...
 

Thanks everyone for your responses. I’ve updated the Review and Commit guidelines to reflect the documentation requirement.  Bin - I think if the PR is large, it’s still best to include the documentation as a separate commit in the same PR, but if it’s in a linked PR that’s not the end of the world.

 

From: Bin Fan <binfan@...>
Date: Thursday, March 4, 2021 at 2:10 PM
To: Rebecca Schlussel <rschlussel@...>
Cc: presto-tsc@... <presto-tsc@...>
Subject: Re: [presto-tsc] Adding doc requirement to commit guidelines

Agree.

 

If the original PR in src code is already large, perhaps we shall allow another separate doc PR linked in the first PR?

 

- Bin

 

On Thu, Mar 4, 2021 at 9:14 AM rschlussel via lists.prestodb.io <rschlussel=fb.com@...> wrote:

Hi everyone,

 

It’s come up a few times recently where new functions have been added without documentation, which makes them not very usable.  Since it’s much easier to ensure that documentation gets written if it’s done when the feature is added, I’d like to propose an update to our commit guidelines to say that new functions, language features, etc. need to have documentation included as part of the PR, and we don’t merge without that documentation.

 

What do others think?

 

Rebecca


 

--

1 - 6 of 6