[dpdk-dev,PATCHv4,5/5] doc: Add ABI __experimental tag documentation
diff mbox

Message ID 20171213151728.16747-6-nhorman@tuxdriver.com
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Neil Horman Dec. 13, 2017, 3:17 p.m. UTC
Document the need to add the __experimental tag to appropriate functions

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Thomas Monjalon <thomas@monjalon.net>
CC: "Mcnamara, John" <john.mcnamara@intel.com>
CC: Bruce Richardson <bruce.richardson@intel.com>
---
 doc/guides/contributing/versioning.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Bruce Richardson Dec. 13, 2017, 3:32 p.m. UTC | #1
On Wed, Dec 13, 2017 at 10:17:28AM -0500, Neil Horman wrote:
> Document the need to add the __experimental tag to appropriate functions
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Thomas Monjalon <thomas@monjalon.net>
> CC: "Mcnamara, John" <john.mcnamara@intel.com>
> CC: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  doc/guides/contributing/versioning.rst | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst
> index 400090628..53f56397e 100644
> --- a/doc/guides/contributing/versioning.rst
> +++ b/doc/guides/contributing/versioning.rst
> @@ -50,6 +50,15 @@ those new APIs and start finding issues with them, new DPDK APIs will be
>  automatically marked as ``experimental`` to allow for a period of stabilization
>  before they become part of a tracked ABI.
>  
> +Note that marking an API as experimental is a two step process.  To mark an API
> +as experimental, the symbols which are desired to be exported must be placed in
> +an EXPERIMENTAL version block in the corresponding libraries' version map
> +script. Secondly, the corresponding definitions of those exported functions, and
> +their forward declarations (in the development header files), must be marked
> +with the __experimental tag (see rte_compat.h).  The DPDK build makefiles
> +preform a check to ensure that the map file and the C code reflect the same
> +list of symbols.
> +

Minor typo s/preform/perform/

>  ABI versions, once released, are available until such time as their
>  deprecation has been noted in the Release Notes for at least one major release
>  cycle. For example consider the case where the ABI for DPDK 2.0 has been
> -- 
> 2.14.3
>
Ferruh Yigit Jan. 11, 2018, 8:06 p.m. UTC | #2
On 12/13/2017 3:17 PM, Neil Horman wrote:
> Document the need to add the __experimental tag to appropriate functions
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Thomas Monjalon <thomas@monjalon.net>
> CC: "Mcnamara, John" <john.mcnamara@intel.com>
> CC: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  doc/guides/contributing/versioning.rst | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst
> index 400090628..53f56397e 100644
> --- a/doc/guides/contributing/versioning.rst
> +++ b/doc/guides/contributing/versioning.rst
> @@ -50,6 +50,15 @@ those new APIs and start finding issues with them, new DPDK APIs will be
>  automatically marked as ``experimental`` to allow for a period of stabilization
>  before they become part of a tracked ABI.
>  
> +Note that marking an API as experimental is a two step process.  To mark an API
> +as experimental, the symbols which are desired to be exported must be placed in
> +an EXPERIMENTAL version block in the corresponding libraries' version map
> +script. Secondly, the corresponding definitions of those exported functions, and
> +their forward declarations (in the development header files), must be marked
> +with the __experimental tag (see rte_compat.h).  The DPDK build makefiles
> +preform a check to ensure that the map file and the C code reflect the same
> +list of symbols.

There are more steps we historically do to mark an API as experimental:
- Add to function header comment experimental for API documentation, preferably
with a warning tag to highlight it:

/**
 * @warning
 * @b EXPERIMENTAL:
....
 */

- If whole APIs in header file are experimental, add same experimental warning
doxygen comment in file comment, again preferably with warning.

- If whole library is experimental, put EXPERIMENTAL tag into maintainers file
as well.

> +
>  ABI versions, once released, are available until such time as their
>  deprecation has been noted in the Release Notes for at least one major release
>  cycle. For example consider the case where the ABI for DPDK 2.0 has been
>
Neil Horman Jan. 11, 2018, 9:29 p.m. UTC | #3
On Thu, Jan 11, 2018 at 08:06:48PM +0000, Ferruh Yigit wrote:
> On 12/13/2017 3:17 PM, Neil Horman wrote:
> > Document the need to add the __experimental tag to appropriate functions
> > 
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Thomas Monjalon <thomas@monjalon.net>
> > CC: "Mcnamara, John" <john.mcnamara@intel.com>
> > CC: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  doc/guides/contributing/versioning.rst | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst
> > index 400090628..53f56397e 100644
> > --- a/doc/guides/contributing/versioning.rst
> > +++ b/doc/guides/contributing/versioning.rst
> > @@ -50,6 +50,15 @@ those new APIs and start finding issues with them, new DPDK APIs will be
> >  automatically marked as ``experimental`` to allow for a period of stabilization
> >  before they become part of a tracked ABI.
> >  
> > +Note that marking an API as experimental is a two step process.  To mark an API
> > +as experimental, the symbols which are desired to be exported must be placed in
> > +an EXPERIMENTAL version block in the corresponding libraries' version map
> > +script. Secondly, the corresponding definitions of those exported functions, and
> > +their forward declarations (in the development header files), must be marked
> > +with the __experimental tag (see rte_compat.h).  The DPDK build makefiles
> > +preform a check to ensure that the map file and the C code reflect the same
> > +list of symbols.
> 
> There are more steps we historically do to mark an API as experimental:
> - Add to function header comment experimental for API documentation, preferably
> with a warning tag to highlight it:
> 
> /**
>  * @warning
>  * @b EXPERIMENTAL:
> ....
>  */
> 
> - If whole APIs in header file are experimental, add same experimental warning
> doxygen comment in file comment, again preferably with warning.
> 
> - If whole library is experimental, put EXPERIMENTAL tag into maintainers file
> as well.
> 
Is that documented somewhere?  I'd like to add this to the same location that it
otherwise is written out.  The above location was the only place in the guide
that I could find reference to experimental markings.

> > +
> >  ABI versions, once released, are available until such time as their
> >  deprecation has been noted in the Release Notes for at least one major release
> >  cycle. For example consider the case where the ABI for DPDK 2.0 has been
> > 
> 
>
Ferruh Yigit Jan. 12, 2018, 11:50 a.m. UTC | #4
On 1/11/2018 9:29 PM, Neil Horman wrote:
> On Thu, Jan 11, 2018 at 08:06:48PM +0000, Ferruh Yigit wrote:
>> On 12/13/2017 3:17 PM, Neil Horman wrote:
>>> Document the need to add the __experimental tag to appropriate functions
>>>
>>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>>> CC: Thomas Monjalon <thomas@monjalon.net>
>>> CC: "Mcnamara, John" <john.mcnamara@intel.com>
>>> CC: Bruce Richardson <bruce.richardson@intel.com>

<...>

>>>  automatically marked as ``experimental`` to allow for a period of stabilization>>>  before they become part of a tracked ABI.

Full sentences for above statement:
"
Since changes to APIs are most likely immediately after their introduction, as
users begin to take advantage of those new APIs and start finding issues with
them, new DPDK APIs will be automatically marked as experimental to allow for a
period of stabilization before they become part of a tracked ABI.
"

This part is not related to this patchset, but it will be hard to maintain above
behavior, "automatically marked" is not automatic now and moving them to stable
after one release is also not automatic. Do you have any suggestion on how to
manage this, do you think can your script be expanded to cover these checks?

>>>  
>>> +Note that marking an API as experimental is a two step process.  To mark an API
>>> +as experimental, the symbols which are desired to be exported must be placed in
>>> +an EXPERIMENTAL version block in the corresponding libraries' version map
>>> +script. Secondly, the corresponding definitions of those exported functions, and
>>> +their forward declarations (in the development header files), must be marked
>>> +with the __experimental tag (see rte_compat.h).  The DPDK build makefiles
>>> +preform a check to ensure that the map file and the C code reflect the same
>>> +list of symbols.
>>
>> There are more steps we historically do to mark an API as experimental:
>> - Add to function header comment experimental for API documentation, preferably
>> with a warning tag to highlight it:
>>
>> /**
>>  * @warning
>>  * @b EXPERIMENTAL:
>> ....
>>  */
>>
>> - If whole APIs in header file are experimental, add same experimental warning
>> doxygen comment in file comment, again preferably with warning.
>>
>> - If whole library is experimental, put EXPERIMENTAL tag into maintainers file
>> as well.
>>
> Is that documented somewhere?  I'd like to add this to the same location that it
> otherwise is written out.  The above location was the only place in the guide
> that I could find reference to experimental markings.

As far as I know how to mark an API as experimental is not documented.
What do you think making a new section for this information?

> 
>>> +
>>>  ABI versions, once released, are available until such time as their
>>>  deprecation has been noted in the Release Notes for at least one major release
>>>  cycle. For example consider the case where the ABI for DPDK 2.0 has been
>>>
>>
>>
Neil Horman Jan. 12, 2018, 2:37 p.m. UTC | #5
On Fri, Jan 12, 2018 at 11:50:12AM +0000, Ferruh Yigit wrote:
> On 1/11/2018 9:29 PM, Neil Horman wrote:
> > On Thu, Jan 11, 2018 at 08:06:48PM +0000, Ferruh Yigit wrote:
> >> On 12/13/2017 3:17 PM, Neil Horman wrote:
> >>> Document the need to add the __experimental tag to appropriate functions
> >>>
> >>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> >>> CC: Thomas Monjalon <thomas@monjalon.net>
> >>> CC: "Mcnamara, John" <john.mcnamara@intel.com>
> >>> CC: Bruce Richardson <bruce.richardson@intel.com>
> 
> <...>
> 
> >>>  automatically marked as ``experimental`` to allow for a period of stabilization>>>  before they become part of a tracked ABI.
> 
> Full sentences for above statement:
> "
> Since changes to APIs are most likely immediately after their introduction, as
> users begin to take advantage of those new APIs and start finding issues with
> them, new DPDK APIs will be automatically marked as experimental to allow for a
> period of stabilization before they become part of a tracked ABI.
> "
> 
> This part is not related to this patchset, but it will be hard to maintain above
> behavior, "automatically marked" is not automatic now and moving them to stable
> after one release is also not automatic. Do you have any suggestion on how to
> manage this, do you think can your script be expanded to cover these checks?
> 

I would make the argument that this was never the case, but rather a statement
of principle.  I assert that because I can find no mechanism anywhere in our
build system that 'automatically' documented or marked a new API as experimental
(please correct me if I'm wrong here).  I think this was more meant to be a
directive to developers to do whatever coding was needed to preform such
marking/documentation in whatever style/format was current.  E.g. introducers of
a new API should document everything as EXPERIMENTAL using the appropriate
doxygen tag and version map tag.

In answer to your question, While we might be able to expand my script to check
for new API's and ensure they are marked as experimental, I don't think thats
the right place to do it, because that script is run at build time, where the
state of the tree is transient. A better place to do it would be with a git hook
at checkin time, or in the checkpatch script to flag new apis as experimental
right before those new API's are comitted.  Though I'm not a huge fan of that
either (at least not of the former suggestion).  I say that because I think we
need to allow developers the freedom to determine the supported status of any
new API that they add.  For example, it seems pretty clear that a new library
might want to have its entire API marked as experimental, but someone adding a
single new function to an existing API might be confident that the new function
is needed and should be immediately supported..

I think the better solution is to define the use of the EXPERIMENTAL tag in the
version map as the canonical location to define unstable API functions.  Doing
so immediately commits an author to ensuring that the corresponding function
definitions are marked with the __experimental tags, which in turn will cause
any downstream users to be alerted to the fact that they might be using those
API's in their code, so they can take appropriate action.  It still allows for
the Documentation to be out of sync, but alerting authors doing development I
think is the more important element here, as Documentation can be corrected more
easily than code in the field.

Thoughts?
Neil

> >>>  
> >>> +Note that marking an API as experimental is a two step process.  To mark an API
> >>> +as experimental, the symbols which are desired to be exported must be placed in
> >>> +an EXPERIMENTAL version block in the corresponding libraries' version map
> >>> +script. Secondly, the corresponding definitions of those exported functions, and
> >>> +their forward declarations (in the development header files), must be marked
> >>> +with the __experimental tag (see rte_compat.h).  The DPDK build makefiles
> >>> +preform a check to ensure that the map file and the C code reflect the same
> >>> +list of symbols.
> >>
> >> There are more steps we historically do to mark an API as experimental:
> >> - Add to function header comment experimental for API documentation, preferably
> >> with a warning tag to highlight it:
> >>
> >> /**
> >>  * @warning
> >>  * @b EXPERIMENTAL:
> >> ....
> >>  */
> >>
> >> - If whole APIs in header file are experimental, add same experimental warning
> >> doxygen comment in file comment, again preferably with warning.
> >>
> >> - If whole library is experimental, put EXPERIMENTAL tag into maintainers file
> >> as well.
> >>
> > Is that documented somewhere?  I'd like to add this to the same location that it
> > otherwise is written out.  The above location was the only place in the guide
> > that I could find reference to experimental markings.
> 
> As far as I know how to mark an API as experimental is not documented.
> What do you think making a new section for this information?
> 
> > 
> >>> +
> >>>  ABI versions, once released, are available until such time as their
> >>>  deprecation has been noted in the Release Notes for at least one major release
> >>>  cycle. For example consider the case where the ABI for DPDK 2.0 has been
> >>>
> >>
> >>
> 
>
Ferruh Yigit Jan. 12, 2018, 3:55 p.m. UTC | #6
On 1/12/2018 2:37 PM, Neil Horman wrote:
> On Fri, Jan 12, 2018 at 11:50:12AM +0000, Ferruh Yigit wrote:
>> On 1/11/2018 9:29 PM, Neil Horman wrote:
>>> On Thu, Jan 11, 2018 at 08:06:48PM +0000, Ferruh Yigit wrote:
>>>> On 12/13/2017 3:17 PM, Neil Horman wrote:
>>>>> Document the need to add the __experimental tag to appropriate functions
>>>>>
>>>>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>>>>> CC: Thomas Monjalon <thomas@monjalon.net>
>>>>> CC: "Mcnamara, John" <john.mcnamara@intel.com>
>>>>> CC: Bruce Richardson <bruce.richardson@intel.com>
>>
>> <...>
>>
>>>>>  automatically marked as ``experimental`` to allow for a period of stabilization>>>  before they become part of a tracked ABI.
>>
>> Full sentences for above statement:
>> "
>> Since changes to APIs are most likely immediately after their introduction, as
>> users begin to take advantage of those new APIs and start finding issues with
>> them, new DPDK APIs will be automatically marked as experimental to allow for a
>> period of stabilization before they become part of a tracked ABI.
>> "
>>
>> This part is not related to this patchset, but it will be hard to maintain above
>> behavior, "automatically marked" is not automatic now and moving them to stable
>> after one release is also not automatic. Do you have any suggestion on how to
>> manage this, do you think can your script be expanded to cover these checks?
>>
> 
> I would make the argument that this was never the case, but rather a statement
> of principle.  I assert that because I can find no mechanism anywhere in our
> build system that 'automatically' documented or marked a new API as experimental
> (please correct me if I'm wrong here).  I think this was more meant to be a
> directive to developers to do whatever coding was needed to preform such
> marking/documentation in whatever style/format was current.  E.g. introducers of
> a new API should document everything as EXPERIMENTAL using the appropriate
> doxygen tag and version map tag.
> 
> In answer to your question, While we might be able to expand my script to check
> for new API's and ensure they are marked as experimental, I don't think thats
> the right place to do it, because that script is run at build time, where the
> state of the tree is transient. A better place to do it would be with a git hook
> at checkin time, or in the checkpatch script to flag new apis as experimental
> right before those new API's are comitted.  Though I'm not a huge fan of that
> either (at least not of the former suggestion).  I say that because I think we
> need to allow developers the freedom to determine the supported status of any
> new API that they add.  For example, it seems pretty clear that a new library
> might want to have its entire API marked as experimental, but someone adding a
> single new function to an existing API might be confident that the new function
> is needed and should be immediately supported..
> 
> I think the better solution is to define the use of the EXPERIMENTAL tag in the
> version map as the canonical location to define unstable API functions.  Doing
> so immediately commits an author to ensuring that the corresponding function
> definitions are marked with the __experimental tags, which in turn will cause
> any downstream users to be alerted to the fact that they might be using those
> API's in their code, so they can take appropriate action.  It still allows for
> the Documentation to be out of sync, but alerting authors doing development I
> think is the more important element here, as Documentation can be corrected more
> easily than code in the field.
> 
> Thoughts?

After this point agree to using EXPERIMENTAL tag in the version map as standard,
but it will be hard to maintain "API is experimental for first release" without
help of any automated tool.


> Neil
> 
>>>>>  
>>>>> +Note that marking an API as experimental is a two step process.  To mark an API
>>>>> +as experimental, the symbols which are desired to be exported must be placed in
>>>>> +an EXPERIMENTAL version block in the corresponding libraries' version map
>>>>> +script. Secondly, the corresponding definitions of those exported functions, and
>>>>> +their forward declarations (in the development header files), must be marked
>>>>> +with the __experimental tag (see rte_compat.h).  The DPDK build makefiles
>>>>> +preform a check to ensure that the map file and the C code reflect the same
>>>>> +list of symbols.
>>>>
>>>> There are more steps we historically do to mark an API as experimental:
>>>> - Add to function header comment experimental for API documentation, preferably
>>>> with a warning tag to highlight it:
>>>>
>>>> /**
>>>>  * @warning
>>>>  * @b EXPERIMENTAL:
>>>> ....
>>>>  */
>>>>
>>>> - If whole APIs in header file are experimental, add same experimental warning
>>>> doxygen comment in file comment, again preferably with warning.
>>>>
>>>> - If whole library is experimental, put EXPERIMENTAL tag into maintainers file
>>>> as well.
>>>>
>>> Is that documented somewhere?  I'd like to add this to the same location that it
>>> otherwise is written out.  The above location was the only place in the guide
>>> that I could find reference to experimental markings.
>>
>> As far as I know how to mark an API as experimental is not documented.
>> What do you think making a new section for this information?
>>
>>>
>>>>> +
>>>>>  ABI versions, once released, are available until such time as their
>>>>>  deprecation has been noted in the Release Notes for at least one major release
>>>>>  cycle. For example consider the case where the ABI for DPDK 2.0 has been
>>>>>
>>>>
>>>>
>>
>>
Neil Horman Jan. 13, 2018, 12:28 a.m. UTC | #7
On Fri, Jan 12, 2018 at 03:55:10PM +0000, Ferruh Yigit wrote:
> On 1/12/2018 2:37 PM, Neil Horman wrote:
> > On Fri, Jan 12, 2018 at 11:50:12AM +0000, Ferruh Yigit wrote:
> >> On 1/11/2018 9:29 PM, Neil Horman wrote:
> >>> On Thu, Jan 11, 2018 at 08:06:48PM +0000, Ferruh Yigit wrote:
> >>>> On 12/13/2017 3:17 PM, Neil Horman wrote:
> >>>>> Document the need to add the __experimental tag to appropriate functions
> >>>>>
> >>>>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> >>>>> CC: Thomas Monjalon <thomas@monjalon.net>
> >>>>> CC: "Mcnamara, John" <john.mcnamara@intel.com>
> >>>>> CC: Bruce Richardson <bruce.richardson@intel.com>
> >>
> >> <...>
> >>
> >>>>>  automatically marked as ``experimental`` to allow for a period of stabilization>>>  before they become part of a tracked ABI.
> >>
> >> Full sentences for above statement:
> >> "
> >> Since changes to APIs are most likely immediately after their introduction, as
> >> users begin to take advantage of those new APIs and start finding issues with
> >> them, new DPDK APIs will be automatically marked as experimental to allow for a
> >> period of stabilization before they become part of a tracked ABI.
> >> "
> >>
> >> This part is not related to this patchset, but it will be hard to maintain above
> >> behavior, "automatically marked" is not automatic now and moving them to stable
> >> after one release is also not automatic. Do you have any suggestion on how to
> >> manage this, do you think can your script be expanded to cover these checks?
> >>
> > 
> > I would make the argument that this was never the case, but rather a statement
> > of principle.  I assert that because I can find no mechanism anywhere in our
> > build system that 'automatically' documented or marked a new API as experimental
> > (please correct me if I'm wrong here).  I think this was more meant to be a
> > directive to developers to do whatever coding was needed to preform such
> > marking/documentation in whatever style/format was current.  E.g. introducers of
> > a new API should document everything as EXPERIMENTAL using the appropriate
> > doxygen tag and version map tag.
> > 
> > In answer to your question, While we might be able to expand my script to check
> > for new API's and ensure they are marked as experimental, I don't think thats
> > the right place to do it, because that script is run at build time, where the
> > state of the tree is transient. A better place to do it would be with a git hook
> > at checkin time, or in the checkpatch script to flag new apis as experimental
> > right before those new API's are comitted.  Though I'm not a huge fan of that
> > either (at least not of the former suggestion).  I say that because I think we
> > need to allow developers the freedom to determine the supported status of any
> > new API that they add.  For example, it seems pretty clear that a new library
> > might want to have its entire API marked as experimental, but someone adding a
> > single new function to an existing API might be confident that the new function
> > is needed and should be immediately supported..
> > 
> > I think the better solution is to define the use of the EXPERIMENTAL tag in the
> > version map as the canonical location to define unstable API functions.  Doing
> > so immediately commits an author to ensuring that the corresponding function
> > definitions are marked with the __experimental tags, which in turn will cause
> > any downstream users to be alerted to the fact that they might be using those
> > API's in their code, so they can take appropriate action.  It still allows for
> > the Documentation to be out of sync, but alerting authors doing development I
> > think is the more important element here, as Documentation can be corrected more
> > easily than code in the field.
> > 
> > Thoughts?
> 
> After this point agree to using EXPERIMENTAL tag in the version map as standard,
> but it will be hard to maintain "API is experimental for first release" without
> help of any automated tool.
> 
I completely agree, in fact I would say it is impossible to do without tooling,
with or without this change.  I think we need to do 1 of 2 things:

1) Add some code to checkpatch.pl to put up a warning if any new apis are added
without marking them as experimental

2) Change the documentation to be a suggestion rather than a requirement.

I'll look into doing (1), but I'm wondering if (2) is the more flexible way to
go. I'm hesitant to enforce the initial marking of new APIs as experimental.
Thoughts?

Neil
Thomas Monjalon Jan. 13, 2018, 3:56 p.m. UTC | #8
13/01/2018 01:28, Neil Horman:
> On Fri, Jan 12, 2018 at 03:55:10PM +0000, Ferruh Yigit wrote:
> > After this point agree to using EXPERIMENTAL tag in the version map as standard,
> > but it will be hard to maintain "API is experimental for first release" without
> > help of any automated tool.
> > 
> I completely agree, in fact I would say it is impossible to do without tooling,
> with or without this change.  I think we need to do 1 of 2 things:
> 
> 1) Add some code to checkpatch.pl to put up a warning if any new apis are added
> without marking them as experimental
> 
> 2) Change the documentation to be a suggestion rather than a requirement.
> 
> I'll look into doing (1), but I'm wondering if (2) is the more flexible way to
> go. I'm hesitant to enforce the initial marking of new APIs as experimental.
> Thoughts?

There will be always cases where we are sure that the experimental step
is not needed.
Even if it is required and checked by a tool, we can ignore it, right?
However, there is no big benefit of bypassing the experimental step.

I am for making mandatory the new API as experimental.
We will handle the exceptions case by case if any.
Neil Horman Jan. 14, 2018, 2:36 p.m. UTC | #9
On Sat, Jan 13, 2018 at 04:56:11PM +0100, Thomas Monjalon wrote:
> 13/01/2018 01:28, Neil Horman:
> > On Fri, Jan 12, 2018 at 03:55:10PM +0000, Ferruh Yigit wrote:
> > > After this point agree to using EXPERIMENTAL tag in the version map as standard,
> > > but it will be hard to maintain "API is experimental for first release" without
> > > help of any automated tool.
> > > 
> > I completely agree, in fact I would say it is impossible to do without tooling,
> > with or without this change.  I think we need to do 1 of 2 things:
> > 
> > 1) Add some code to checkpatch.pl to put up a warning if any new apis are added
> > without marking them as experimental
> > 
> > 2) Change the documentation to be a suggestion rather than a requirement.
> > 
> > I'll look into doing (1), but I'm wondering if (2) is the more flexible way to
> > go. I'm hesitant to enforce the initial marking of new APIs as experimental.
> > Thoughts?
> 
> There will be always cases where we are sure that the experimental step
> is not needed.
> Even if it is required and checked by a tool, we can ignore it, right?
> However, there is no big benefit of bypassing the experimental step.
> 
> I am for making mandatory the new API as experimental.
> We will handle the exceptions case by case if any.
> 
If the consensus is to require experimental marking by default, and grant
exceptions as needed, then I would strongly suggest that we do this in
checkpatch as I can modify it to warn people of added API's (which will be
reflected in the CI tool, if the CI group is still maintaining it), but we can
collectively ignore it if its so clearly trivial that it requires no
experimental addition (which I think may freqently be the case).

I'll start work on that on monday

Best
Neil
Thomas Monjalon Jan. 14, 2018, 4:27 p.m. UTC | #10
14/01/2018 15:36, Neil Horman:
> On Sat, Jan 13, 2018 at 04:56:11PM +0100, Thomas Monjalon wrote:
> > 13/01/2018 01:28, Neil Horman:
> > > On Fri, Jan 12, 2018 at 03:55:10PM +0000, Ferruh Yigit wrote:
> > > > After this point agree to using EXPERIMENTAL tag in the version map as standard,
> > > > but it will be hard to maintain "API is experimental for first release" without
> > > > help of any automated tool.
> > > > 
> > > I completely agree, in fact I would say it is impossible to do without tooling,
> > > with or without this change.  I think we need to do 1 of 2 things:
> > > 
> > > 1) Add some code to checkpatch.pl to put up a warning if any new apis are added
> > > without marking them as experimental
> > > 
> > > 2) Change the documentation to be a suggestion rather than a requirement.
> > > 
> > > I'll look into doing (1), but I'm wondering if (2) is the more flexible way to
> > > go. I'm hesitant to enforce the initial marking of new APIs as experimental.
> > > Thoughts?
> > 
> > There will be always cases where we are sure that the experimental step
> > is not needed.
> > Even if it is required and checked by a tool, we can ignore it, right?
> > However, there is no big benefit of bypassing the experimental step.
> > 
> > I am for making mandatory the new API as experimental.
> > We will handle the exceptions case by case if any.
> > 
> If the consensus is to require experimental marking by default, and grant
> exceptions as needed, then I would strongly suggest that we do this in
> checkpatch as I can modify it to warn people of added API's (which will be
> reflected in the CI tool, if the CI group is still maintaining it), but we can
> collectively ignore it if its so clearly trivial that it requires no
> experimental addition (which I think may freqently be the case).

I am OK with this approach.

> I'll start work on that on monday

Good

I wonder how difficult it can be to implement.
Note: we do not maintain a fork of checkpatch.pl.
We just have a shell wrapper checkpatch.sh.

Maybe you should start a different tool.
Can it use Coccinelle?
Thomas Monjalon Jan. 21, 2018, 8:14 p.m. UTC | #11
13/12/2017 16:17, Neil Horman:
> --- a/doc/guides/contributing/versioning.rst
> +++ b/doc/guides/contributing/versioning.rst
> +Note that marking an API as experimental is a two step process.  To mark an API
> +as experimental, the symbols which are desired to be exported must be placed in
> +an EXPERIMENTAL version block in the corresponding libraries' version map
> +script. Secondly, the corresponding definitions of those exported functions, and
> +their forward declarations (in the development header files), must be marked
> +with the __experimental tag (see rte_compat.h).  The DPDK build makefiles
> +preform a check to ensure that the map file and the C code reflect the same
> +list of symbols.

Thanks for this text.

Bruce already commented about the type "preform".

Ferruh already commented about adding a string in doxygen header.

Ferruh already commented about adding sentences for new API.

I add that it would be the right place to explain the effect of the
attribute, and how it can be disabled at compilation.

Patch
diff mbox

diff --git a/doc/guides/contributing/versioning.rst b/doc/guides/contributing/versioning.rst
index 400090628..53f56397e 100644
--- a/doc/guides/contributing/versioning.rst
+++ b/doc/guides/contributing/versioning.rst
@@ -50,6 +50,15 @@  those new APIs and start finding issues with them, new DPDK APIs will be
 automatically marked as ``experimental`` to allow for a period of stabilization
 before they become part of a tracked ABI.
 
+Note that marking an API as experimental is a two step process.  To mark an API
+as experimental, the symbols which are desired to be exported must be placed in
+an EXPERIMENTAL version block in the corresponding libraries' version map
+script. Secondly, the corresponding definitions of those exported functions, and
+their forward declarations (in the development header files), must be marked
+with the __experimental tag (see rte_compat.h).  The DPDK build makefiles
+preform a check to ensure that the map file and the C code reflect the same
+list of symbols.
+
 ABI versions, once released, are available until such time as their
 deprecation has been noted in the Release Notes for at least one major release
 cycle. For example consider the case where the ABI for DPDK 2.0 has been