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

[Rodrigo Vivi]

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.

> +
> +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?

> 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.

> +
> +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.

> +
> +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.

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

> +
> +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.

> 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....

> +
> +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/
- mention to dim push-fixes-next
- explicit gvt next for pull request on next-queued.

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

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
>