Re: [dpdk-dev] [PATCH v7] doc: add release milestones definition

[Thomas Monjalon <thomas@monjalon.net>]

19/05/2021 13:58, Ferruh Yigit:
> On 5/18/2021 5:43 PM, Thomas Monjalon wrote:
> > From: Asaf Penso <asafp@nvidia.com>
> > 
> > Adding more information about the release milestones.
> > This includes the scope of change, expectations, etc.
> > 
> > Signed-off-by: Asaf Penso <asafp@nvidia.com>
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > Acked-by: John McNamara <john.mcnamara@intel.com>
> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > ---
> > v2: fix styling format and add content in the commit message
> > v3: change punctuation and avoid plural form when unneeded
> > v4: avoid abbreviations, "Priority" in -rc, and reword as John suggests
> > v5: note that release candidates may vary
> > v6: merge RFC and proposal deadline, add roadmap link and reduce duplication
> > v7: make expectations clearer and stricter
> > ---
> >  doc/guides/contributing/patches.rst | 72 +++++++++++++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> > 
> > diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
> > index 6dbbd5f8d1..4d70e326c5 100644
> > --- a/doc/guides/contributing/patches.rst
> > +++ b/doc/guides/contributing/patches.rst
> > @@ -177,6 +177,8 @@ Make your planned changes in the cloned ``dpdk`` repo. Here are some guidelines
> >  * Add documentation, if relevant, in the form of Doxygen comments or a User Guide in RST format.
> >    See the :ref:`Documentation Guidelines <doc_guidelines>`.
> >  
> > +* Code and related documentation must be updated atomically in the same patch.
> > +
> >  Once the changes have been made you should commit them to your local repo.
> >  
> >  For small changes, that do not require specific explanations, it is better to keep things together in the
> > @@ -660,3 +662,73 @@ patch accepted. The general cycle for patch review and acceptance is:
> >       than rework of the original.
> >     * Trivial patches may be merged sooner than described above at the tree committer's
> >       discretion.
> > +
> > +
> > +Milestones definition
> > +---------------------
> > +
> > +Each DPDK release has milestones that help everyone to converge to the release date.
> > +The following is a list of these milestones
> > +together with concrete definitions and expectations,
> 
> Is the ',' correct here. John may help better.
> 
> > +for a typical release cycle (3 months ending after 4 release candidates).
> 
> Can it be better to put explicitly that a regular release cycle takes around 3
> months and it mostly has 4 release candidates, instead of giving this shortly
> within parenthesis?
> 
> > +The number and expectations of release candidates might vary slightly.
> > +The schedule is updated in the `roadmap <https://core.dpdk.org/roadmap/#dates>`_.
> > +
> > +.. note::
> > +   Sooner is always better. Deadlines are not ideal dates.
> > +
> 
> +1
> 
> > +   Integration is never guaranteed but everyone can help.
> > +
> > +Roadmap
> > +~~~~~~~
> > +
> > +* Announce new features in libraries, drivers, applications, and examples.
> 
> Do we need to call out the components, what about "Announce new features for
> next release."

The idea was to insist that all can be announced, including apps.

> Is it fair to say features announced with a roadmap will get more priority
> against the features that are not announced?

It should be discussed in a separate patch I think.

> > +* To be published before the first day of the release cycle.
> > +
> > +Proposal Deadline
> 
> Does it have any value to document something like this:
> 
> release start - proposal deadline: Initial development phase
> proposal deadline - rc1: review / update / review phase
> rc1 - release: test / update / test phase

I don't know.

> > +~~~~~~~~~~~~~~~~~
> > +
> > +* Must send an RFC or a complete v1 patch.
> 
> The subject is missing, it is developers, but would it be better to say
> something like:
> 
> An RFC (Request For Comment) or first version of the implementation must be
> ready before deadline for the feature to be taken into account for the release.

I was trying to keep it short.

> > +* Early RFC gives time for design review before complete implementation.
> > +* Should include at least the API changes in libraries and applications.
> 
> Again subject is missing,
> 
> Implementation should include ...

I'm afraid it will become boring.

> > +* Library code should be quite complete at the deadline.
> > +* Nice to have: driver implementation (full or draft), example code, and documentation.
> 
> This is talking about driver/example/documentation update related to a library
> implementation, right?

Right

> Otherwise for the standalone driver/example implementation, isn't the target to
> send them before proposal deadline?

Yes

> > +
> > +rc1
> > +~~~
> > +
> > +* Priority: libraries. No library feature should be accepted after -rc1.
> > +* New API must be defined and implemented in libraries.
> 
> Not sure about the value of "defined", why not just "new API must be implemented
> in libraries". Also I guess it doesn't need to 'new'.

OK

> Also another big step here is it needs to approved by its maintainers.
> 
> What about something like:
> 
> API changes or additions must be implemented and approved by their maintainers.

Yes good idea.

> And as we know reviews can be bottleneck perhaps we can add a note on that too,
> like: Developer should send a reminder for the patches that has not reviewed for
> more than two weeks. ??

Yes, 10 days?

> > +* The API must include Doxygen documentation
> > +  and be part of the relevant .rst files (library-specific and release notes).
> > +* API should be used in a test application (``/app``).
> > +* At least one PMD should implement the API.
> > +  It may be a draft sent in a separate series.
> 
> Above 2/3 bullets are the conditions for a feature to be merged, it is not
> directly related to the milestone, should we have separate section to document
> the expectation to upstream a new API?

Yes, you're right but I feel it has a better value here as a complete checklist.

> > +* The above should be sent to the mailing list at least 2 weeks before -rc1.
> > +* Nice to have: example code (``/examples``)
> > +
> 
> I think we should mention that we expect test result from community after -rc1,
> if this is to document the process.

We expect tests after each -rc, so it would better fit as a general comment
in the schedule introduction.

> > +rc2
> > +~~~
> > +
> > +* Priority: drivers. No driver feature should be accepted after -rc2.
> > +* A driver change must include documentation
> > +  in the relevant .rst files (driver-specific and release notes).
> > +* The above should be sent to the mailing list at least 2 weeks before -rc2.
> > +
> 
> I think testing and fixing issues found in the -rc1 is the big focus of the
> -rc2. But we don't mention of the testing at all. Again I guess this is based on
> the confusion if these descriptions define process or todo list for the developers.

I would like to have both at the same place,
so I agree we should mention fixing any issue found in previous stages.

> > +rc3
> > +~~~
> > +
> > +* Priority: applications. No application feature should be accepted after -rc3.
> > +* New functionality that does not depend on libraries update
> > +  can be integrated as part of -rc3.
> > +* The application change must include documentation in the relevant .rst files
> > +  (application-specific and release notes if significant).
> > +* Libraries and drivers cleanup are allowed.
> > +* Small driver reworks.
> > +* Critical and minor bug fixes.
> 
> Can this cause misunderstanding that fixes are merged for -rc3 (after -rc2)?
> Can we highlight that fixes can be sent and merged anytime until -rc4, only
> after -rc4 there is a restriction and only critical fixes are accepted?

I think we should mention "any fix" for -rc2,
and "all accepted" for -rc1 to remove confusion.

> > +
> > +rc4
> > +~~~
> > +
> > +* Documentation updates.
> > +* Critical bug fixes.