From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luca Boccassi Subject: Re: [PATCHv4 0/4] dpdk: enhance EXPERIMENTAL api tagging Date: Sat, 30 Dec 2017 20:20:58 +0100 Message-ID: <1514661658.6638.59.camel@debian.org> References: <20171201185628.16261-1-nhorman@tuxdriver.com> <20171213151728.16747-1-nhorman@tuxdriver.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable To: Neil Horman , dev@dpdk.org Return-path: Received: from mail-wr0-f180.google.com (mail-wr0-f180.google.com [209.85.128.180]) by dpdk.org (Postfix) with ESMTP id D83F7160 for ; Sat, 30 Dec 2017 20:21:00 +0100 (CET) Received: by mail-wr0-f180.google.com with SMTP id v21so33261242wrc.0 for ; Sat, 30 Dec 2017 11:21:00 -0800 (PST) In-Reply-To: <20171213151728.16747-1-nhorman@tuxdriver.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, 2017-12-13 at 10:17 -0500, Neil Horman wrote: > Hey all- > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0A few days ago, I was lam= enting the fact that, when reviewing > patches I > would frequently complain about ABI changes that were actually > considered safe > because they were part of the EXPERIMENTAL api set.=C2=A0=C2=A0John M. as= ked me > then what > I might do to improve the situation, and the following patch set is a > proposal > that I've come up with. >=20 > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0In thinking about the pro= blem I identified two issues that I > think we > can improve on in this area: >=20 > 1) Make experimental api calls more visible in the source code.=C2=A0=C2= =A0That > is to say, > when reviewing patches, it would be nice to have some sort of visual > reference > that indicates that the changes being made are part of an > experimental API and > therefore ABI concerns need not be addressed >=20 > 2) Make experimenal api usage more visible to consumers of the DPDK, > so that > they can make a more informed decision about the API's they consume > in their > application.=C2=A0=C2=A0We make an effort to document all the experimenta= l > API's, but > there is no guarantee that a user will check the documentation before > making use > of a new library. >=20 > This patch set attempts to achieve both of the above goals.=C2=A0=C2=A0To= do > this I've > added an __experimental macro tag, suitable for inserting into api > forward > declarations and definitions. >=20 > The presence of the tag in the header and c files where the api code > resides > increases the likelyhood that any patch submitted against them will > include the > tag in the context, making it clear to reviewers that ABI stability > isn't a > concern here. >=20 > Also, This tag marks each function it is used on with an attibute > causing any > use of the fuction to emit a warning during the build > with a message indicating that the API call in question is not yet > part of the > stable interface.=C2=A0=C2=A0Developers can then make an informed decisio= n to > suppress > that warning or not. >=20 > Because there is internal use of several experimental API's, this set > also > includes a new override macro ALLOW_EXPERIMENTAL_APIS to > automatically > suprress these warnings.=C2=A0=C2=A0I think its fair to assume that, for > internal use, we > almost always want to suppress these warnings, as by definition any > change to > the apis (even their removal) must be done in parallel with an > appropriate > change in the calling locations, lest the dpdk build itself break. >=20 > Neil >=20 > --- > Change Notes: > v2) > * Cleaned up checkpatch errors > * Added Allowance for building experimental on BSD > * Swapped Patch 3 and 4 so that we didn't have a commit level that > issued > =C2=A0 warnings/errors without need >=20 > v3) > * On suggestion from Bruce, modify ALLOW_EXPERIMENTAL_APIS to be > defined in > =C2=A0 CFLAGS rather than a makefile variable.=C2=A0=C2=A0This is more fl= exible in > that it > =C2=A0 allows us to suppress this specific feature rather than all uses o= f > the > =C2=A0 deprecated attribute, as we might use it for other features in the > furute >=20 > v4) > * Added documentation patch to contributors guide Acked-by: Luca Boccassi I really like the idea of showing warnings at build time for users of the libraries, in fact I like it so much I'm going to shamelessly steal it for another few projects I work on where we have an experimental (DRAFT) api system :-) --=20 Kind regards, Luca Boccassi