[PATCH 09/11] doc: add common maintainer guidelines file, start with drm-intel

[Jani Nikula]

On Thu, 25 Oct 2018, Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
> On Thu, Oct 25, 2018 at 06:22:09PM +0300, Jani Nikula wrote:
>> For starters, add the specific guidelines in their own files, starting
>> with drm-intel, with the intention of consolidating and pulling in
>> common guides in the top level file in the long term.
>> 
>> Start with adding detailed maintainer tasks for handling drm-intel
>> branches.
>> 
>> v2: some tweaks, use dim rebase for rebases
>> 
>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>> ---
>>  index.rst                 |   1 +
>>  maintainer-drm-intel.rst  | 141 ++++++++++++++++++++++++++++++++++++++++++++++
>>  maintainer-guidelines.rst |  12 ++++
>>  3 files changed, 154 insertions(+)
>>  create mode 100644 maintainer-drm-intel.rst
>>  create mode 100644 maintainer-guidelines.rst
>> 
>> diff --git a/index.rst b/index.rst
>> index 88b49edbeece..7049d94ccc18 100644
>> --- a/index.rst
>> +++ b/index.rst
>> @@ -27,6 +27,7 @@ Contents:
>>     drm-misc
>>     drm-intel
>>     committer-guidelines
>> +   maintainer-guidelines
>>     commit-access
>>     getting-started
>>     dim
>> diff --git a/maintainer-drm-intel.rst b/maintainer-drm-intel.rst
>> new file mode 100644
>> index 000000000000..82a3a739eccb
>> --- /dev/null
>> +++ b/maintainer-drm-intel.rst
>> @@ -0,0 +1,141 @@
>> +.. _maintainer-drm-intel:
>> +
>> +=================================
>> + drm-intel Maintainer Guidelines
>> +=================================
>> +
>> +This document describes the detailed maintainer tasks for drm-intel.
>> +
>> +Maintaining drm-intel Branches
>> +==============================
>> +
>> +The guide assumes a maintainer rotation of one person handling features and
>> +fixes for one release. Thus for each branch, you take over from the person
>> +maintaining the branch before you, and leave it in a known state for the person
>> +after you.
>> +
>> +drm-intel-next-queued
>> +---------------------
>> +
>> +Take over when the last drm-intel feature pull has been sent for an upcoming
>> +merge window, and drm-intel-next-queued starts targeting the next merge window
>> +after that. This happens around -rc5 of the current development kernel.
>> +
>> +Tag drm-intel-next-queued periodically, say every 1-2 weeks, depending on the
>> +rate of change. Use ``dim update-next``. Write a tag summary detailing the
>> +changes since the last tag. Send a testing request.
>> +
>> +Send drm-intel-next-queued pull requests periodically. Use ``dim
>> +pull-request-next``. Pull requests are based on the tags generated above. Don't
>> +send a pull request immediately after tagging, leave some time for
>> +testing. Typically you would send a pull request for each tag, but the tooling
>> +allows for accumulation of several tags into one pull request. Dave usually
>> +doesn't start pulling features until the pull requests for the previous release
>> +have been merged to Linus' tree, so it's normal to accumulate several tags
>> +before and during the merge window. Only send your first pull request after
>> +-rc1.
>
> Probably the window -rc1 to -rc5 requires a dedicated section.

What do you mean?

>
>> +
>> +Do backmerges as needed, but also don't let drm-intel-next-queued fall too much
>> +behind from drm-next. Use ``dim backmerge``.
>
> well, I remember being told something like: "only do backmerges when there is a
> real reason/need and make sure this is documented on backmerge commit"
>
> I mean, I'm not against what is written now, but I believe the "fall too much
> behind" is kind of vague. Should we do at least one backmerge at some specific
> point in time to avoid getting too behind?

Maybe I'll just add "Use your discretion" because that's what it will
boil down to.

>
>> Never rebase or force push. Only do
>> +backmerges from drm-next. Specifically, don't merge drm-misc branches or
>> +backmerge Linus' tree directly; they all need to go through drm-next. Request
>> +drm-misc maintainers to have your dependencies sent to drm-next, and request
>> +Dave to have Linus' tree backmerged to drm-next.
>> +
>> +Pull in gvt pull requests. Use ``dim apply-pull``.
>
> I believe we need to clarify that this is talking about gvt pull targeting next.

Ok.

>
>> +
>> +Usually committers handle most patches, but look for the patches that fall
>> +through the cracks otherwise. Ping for reviews, and push reviewed patches.
>> +
>> +Track the patches being committed, ensuring the merge criteria and the rest of
>> +the process is being properly followed.
>> +
>> +Remind people about the last feature pull deadline (-rc5 time frame), especially
>> +when there are known features aiming for a release. It's not the maintainer's
>> +job to ensure deadlines are met, but to raise awareness. Avoid surprising
>> +people.
>
> Besides moving the -rc5 to the dedicated window -rc1-to--rc5 section I wonder
> if we shouldn't start this document with a section for maintainers mission
> or roles and responsibilities that would cover the rest of this phrase here.

Hey, I'm just trying hard to get some of these notes merged before they
got forgotten again. It's been eons since I lost my first notes, and
even since I pasted these for comments. We (and you! ;) can build on
this later!

>
>> +
>> +When sending the last feature pull request, pass the torch to the next in
>> +rotation to maintain drm-intel-next-queued. Move on to drm-intel-next-fixes to
>> +follow through with the features you handled.
>> +
>> +drm-intel-next-fixes
>> +--------------------
>> +
>> +After the last drm-intel feature pull request has been merged to drm-next,
>> +somewhere around -rc6 or -rc7 time frame, rebase drm-intel-next-fixes on top of
>> +drm-next, and push. Use ``dim rebase``.
>> +
>> +Cherry pick fixes from drm-intel-next-queued periodically. Use ``dim
>> +cherry-pick-next-fixes``. The cherry-picks are automated based on Fixes: and Cc:
>> +stable tags. Double check that they make sense. Use ``dim push-next-fixes`` to
>> +push. Stop cherry-picking during the merge window, and only pick the fixes that
>> +really need to make it to -rc1.
>
> I believe it is worth to explicitly recommend at least a compilation check.

Now that's one of those things that rubs me in the wrong way. It should
go without saying you don't push without building. But if you write it
down, it becomes the norm that's enough, and relieves you of anything
beyond that...

The more you add here, the less likely it is that it'll get read.

> Another thing that I miss here is an explicit mention
> to push branch with ``dim push-next-fixes```

It's right there...

>
>> +
>> +Note: ``dim cherry-pick-next-fixes`` and ``dim cherry-pick-fixes`` (see below)
>> +use the same logic to pick patches. The former catches everything that the
>> +latter catches. Both skip commits that have already been picked. Basically
>> +there's a race condition where a commit could be picked for next-fixes when it
>> +should really be headed for fixes. Communicate with the current -fixes
>> +maintainer.
>> +
>> +Send drm-intel-next-fixes pull requests as needed. Check the CI results, and use
>> +``dim pull-request-next-fixes``. The fixes will be merged to drm-next, on top of
>> +the features heading for the next merge window.
>> +
>> +You can send the last pull also during the merge window, even after the main drm
>> +pull request has been sent to Linus. Either there will be a follow-up fixes
>> +pull, or it will be sent and merged after -rc1 as fixes.
>> +
>> +Pull in the (relatively rare) gvt next-fixes pull requests. Use ``dim
>> +apply-pull``.
>> +
>> +Do occasional fast-forward rebases on top of drm-next as needed.
>> +
>> +Make sure everything you've picked has been merged upstream. If not, request
>> +Dave to merge the missing pull requests to drm-fixes after -rc1.
>> +
>> +After -rc1, move on to drm-intel-fixes.
>> +
>> +drm-intel-fixes
>> +---------------
>> +
>> +When you take over, drm-intel-fixes should be at the latest upstream kernel
>> +release tag. After -rc1, rebase drm-intel-fixes on top of it. Rebase on the tags
>> +only, don't rebase on arbitrary commits in Linus' master. Use ``dim rebase
>> +drm-intel-fixes vX.Y-rc1``.
>> +
>> +Adopt a weekly routine for -rc fixes.
>> +
>> +On Monday, if the the last fixes pull has been merged to Linus' tree, rebase
>> +drm-intel-fixes on top as a fast-forward rebase. Again, use ``dim rebase
>> +drm-intel-fixes vX.Y-rcN``. Skip or abort the rebase if it's not a fast-forward
>> +rebase, i.e. some of the cherry-picks have not been merged upstream. Try again
>> +next week.
>> +
>> +After the rebase, give CI time to run the plain upstream tag, to detect issues
>> +introduced by Linus' upstream.
>
> s/After the rebase, give CI/After the rebase, push ``dim push-fixes``. Then, give CI/
>
>> +
>> +Before Wednesday,
>
> on my timezone ideal is tuesday end of the day or wed first thing on the morning.

i.e. *before* wednesday ;)

>
>> cherry pick fixes from drm-intel-next-queued. Use ``dim
>> +cherry-pick-fixes``. The cherry-picks are automated based on Fixes: and Cc:
>> +stable tags. Double check that they make sense. Be more and more critical toward
>> +the higher -rc, nearing stable kernel rules for commits, and drop commits that
>> +are not needed using ``git rebase -i``. Remember, this is the fastest path to
>> +Linus' upstream. Use ``dim push-fixes`` to push.
>
> Oh, it is here. I think we should move above....

What is here?

>
>> +
>> +If cherry-picks fail, look into the failures, and see if the commits are
>> +needed. Send mail to intel-gfx and the authors to ask for help and/or backports
>> +as needed.
>> +
>> +Pull in the gvt fixes pull requests. Use ``dim apply-pull``.
>> +
>> +Again, let CI crunch through the branch. If everything looks good, send the
>> +fixes pull request Thursday at the latest. Due to time zones, a pull request
>> +sent on Friday might not make it, missing the Sunday -rc. Use ``dim
>> +pull-request-fixes``.
>
> On my timezone the ideal is Wed by end of day. Thursday first thing on the day
> as the last resource.
>
>> +
>> +After vX.Y release, rebase drm-intel-fixes one last time on that. Use ``dim
>> +rebase drm-intel-fixes vX.Y``.
>> +
>> +It's time to take a break from maintaining branches,
>
> I just started this sweet spot :)
>
> From all my comments above I believe the critical ones are:
>
> - s/After the rebase, give CI/After the rebase, push ``dim push-fixes``. Then, give CI/

It's there.

> - mention to dim push-fixes-next

It's there.

> - explicit gvt next for pull request on next-queued.

Okay.

>
> With this in place:
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
> Secondary:
>
> - probably a mention/warn to different timezones (but no idea how to do it)
> - rc1 to rc5 window exclusive section

I don't know what should be there.

>
> All rest is bikesheds that could or couldn't be done later.
>
> Thanks for this great doc!
>
> Rodrigo.
>
>> until you start over with
>> +drm-intel-next-queued.
>> diff --git a/maintainer-guidelines.rst b/maintainer-guidelines.rst
>> new file mode 100644
>> index 000000000000..335d29a6e2bb
>> --- /dev/null
>> +++ b/maintainer-guidelines.rst
>> @@ -0,0 +1,12 @@
>> +.. _maintainer-guidelines:
>> +
>> +=======================
>> + Maintainer Guidelines
>> +=======================
>> +
>> +This document gathers together maintainer guidelines.
>> +
>> +.. toctree::
>> +   :maxdepth: 2
>> +
>> +   maintainer-drm-intel
>> -- 
>> 2.11.0
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center