API Review Policy

[Greg Mann <gr...@mesosphere.io>]

Hi all,
As BenM mentioned in a recent email, we discussed our process for review of
API changes in the recent API working group meeting [1]. James Peach was
also kind enough to share with me Apache Traffic Server's API review
policy, which they have formalized [2].

I like the idea of putting our review policy in writing, as this increases
transparency and helps to communicate the policy to new contributors. If
others in the project are amenable to this idea, I could propose a draft
policy and present it to the mailing list for review.

What do folks think about this?

Cheers,
Greg

[1]
https://docs.google.com/document/d/1JrF7pA6gcBZ6iyeP5YgDG62ifn0cZIBWw1f_Ler6fLM/edit
[2] https://cwiki.apache.org/confluence/display/TS/API+Review+Process

Re: API Review Policy

[James Peach <jp...@apache.org>]

> On Mar 13, 2018, at 3:58 PM, Greg Mann <gr...@mesosphere.io> wrote:
> 
> Sure we can use that as a starting point. The basic policy that we
> discussed in the working group was that any public API change should be
> advertised on the developer mailing list. If no concerns are raised after
> several days, then the change could proceed. If concerns were raised, then
> discussion could start on the mailing list and continue in the next meeting
> of the API working group if necessary.
> 
> The Traffic Server policy includes a few other concrete details which we
> did not discuss. I'll quote it here to make things easy:
> 
> 
> Due to the importance of getting API right, there is a required review
>> process for changes to the Traffic Server API. For every API change, the
>> developer should post a message to the dev@ list that
>> 
>>   - references the relevant Github Issue or PR
>>   - explains the motivating problem and rationale
>>   - shows the actual API change itself (ie. API signatures, etc)
>>   - documents the semantics of the proposed API
>>   - notes any ABI or compatibility implicates
>> 
>> After a comments period (1 or 2 days), the committer would add the API. If
>> there were any comments or suggestions, then the committer would address
>> those as necessary.
>> 
>> New API can be added to experimental.h if the developer believe that it
>> might change after some adoption or implementation experience. APIs
>> intended for experimental.h should still be reviewed on the mailing list.
>> APIs added to experimental.h, or another experimental header, can (and
>> should!) get moved to a frozen and stable include file when appropriate.
>> It's up to the author to propose a promotion to stable on the mailing list,
>> lazy consensus applies here.
>> 
>> It is strongly preferable that a new API to be integrated into a sample
>> plugin - giving users a good sample to copy. API documentation and unit
>> tests should be provided as a matter of course.
>> 
>> 
> 
> I think this is pretty good as-is; replace "Github Issue or PR" with "JIRA
> issue or ReviewBoard patch", and remove the portion about 'experimental.h'
> and what follows.

I’m generally in favor of this. I think that we all try to raise compatibility and operational issues on the mailing lists, so this seems like a formalization and extension of that practice. Most of the information needed for an API proposal would already be captured in a design document, so in the Mesos context this would be about improving the visibility of changes and widening the feedback net.

cheers,
James.

Re: API Review Policy

[Greg Mann <gr...@mesosphere.io>]

Sure we can use that as a starting point. The basic policy that we
discussed in the working group was that any public API change should be
advertised on the developer mailing list. If no concerns are raised after
several days, then the change could proceed. If concerns were raised, then
discussion could start on the mailing list and continue in the next meeting
of the API working group if necessary.

The Traffic Server policy includes a few other concrete details which we
did not discuss. I'll quote it here to make things easy:


Due to the importance of getting API right, there is a required review
> process for changes to the Traffic Server API. For every API change, the
> developer should post a message to the dev@ list that
>
>    - references the relevant Github Issue or PR
>    - explains the motivating problem and rationale
>    - shows the actual API change itself (ie. API signatures, etc)
>    - documents the semantics of the proposed API
>    - notes any ABI or compatibility implicates
>
> After a comments period (1 or 2 days), the committer would add the API. If
> there were any comments or suggestions, then the committer would address
> those as necessary.
>
> New API can be added to experimental.h if the developer believe that it
> might change after some adoption or implementation experience. APIs
> intended for experimental.h should still be reviewed on the mailing list.
> APIs added to experimental.h, or another experimental header, can (and
> should!) get moved to a frozen and stable include file when appropriate.
> It's up to the author to propose a promotion to stable on the mailing list,
> lazy consensus applies here.
>
> It is strongly preferable that a new API to be integrated into a sample
> plugin - giving users a good sample to copy. API documentation and unit
> tests should be provided as a matter of course.
>
>

I think this is pretty good as-is; replace "Github Issue or PR" with "JIRA
issue or ReviewBoard patch", and remove the portion about 'experimental.h'
and what follows.

Greg


On Tue, Mar 13, 2018 at 3:40 PM, Zhitao Li <zh...@gmail.com> wrote:

> +1 for keeping the policy in writing. Are we looking at the Traffic
> Server's API policy as a starting point for discussion?
>
> On Tue, Mar 13, 2018 at 10:12 AM, Greg Mann <gr...@mesosphere.io> wrote:
>
> > Hi all,
> > As BenM mentioned in a recent email, we discussed our process for review
> of
> > API changes in the recent API working group meeting [1]. James Peach was
> > also kind enough to share with me Apache Traffic Server's API review
> > policy, which they have formalized [2].
> >
> > I like the idea of putting our review policy in writing, as this
> increases
> > transparency and helps to communicate the policy to new contributors. If
> > others in the project are amenable to this idea, I could propose a draft
> > policy and present it to the mailing list for review.
> >
> > What do folks think about this?
> >
> > Cheers,
> > Greg
> >
> > [1]
> > https://docs.google.com/document/d/1JrF7pA6gcBZ6iyeP5YgDG62ifn0cZ
> > IBWw1f_Ler6fLM/edit
> > [2] https://cwiki.apache.org/confluence/display/TS/API+Review+Process
> >
>
>
>
> --
> Cheers,
>
> Zhitao Li
>

Re: API Review Policy

[Zhitao Li <zh...@gmail.com>]

+1 for keeping the policy in writing. Are we looking at the Traffic
Server's API policy as a starting point for discussion?

On Tue, Mar 13, 2018 at 10:12 AM, Greg Mann <gr...@mesosphere.io> wrote:

> Hi all,
> As BenM mentioned in a recent email, we discussed our process for review of
> API changes in the recent API working group meeting [1]. James Peach was
> also kind enough to share with me Apache Traffic Server's API review
> policy, which they have formalized [2].
>
> I like the idea of putting our review policy in writing, as this increases
> transparency and helps to communicate the policy to new contributors. If
> others in the project are amenable to this idea, I could propose a draft
> policy and present it to the mailing list for review.
>
> What do folks think about this?
>
> Cheers,
> Greg
>
> [1]
> https://docs.google.com/document/d/1JrF7pA6gcBZ6iyeP5YgDG62ifn0cZ
> IBWw1f_Ler6fLM/edit
> [2] https://cwiki.apache.org/confluence/display/TS/API+Review+Process
>



-- 
Cheers,

Zhitao Li