[PATCH 8/8] docs: propose new rules for topic/core-for-CI management

[Tvrtko Ursulin]

On 25/10/2022 11:20, Jani Nikula wrote:
> Require maintainer ack for rebase.
> 
> Require maintainer/committer ack for adding/removing commits.
> 
> Require gitlab issue for each new commit.
> 
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>   drm-tip.rst | 23 ++++++++++++++++++-----
>   1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drm-tip.rst b/drm-tip.rst
> index deae95cdd2fe..fde62feca296 100644
> --- a/drm-tip.rst
> +++ b/drm-tip.rst
> @@ -203,11 +203,13 @@ justified exception. The primary goal is to fix issues originating from Linus'
>   tree. Issues that would need drm-next or other DRM subsystem tree as baseline
>   should be fixed in the offending DRM subsystem tree.
>   
> -Only rebase the branch if you really know what you're doing. When in doubt, ask
> -the maintainers. You'll need to be able to handle any conflicts in non-drm code
> -while rebasing.
> +Only rebase the branch if you really know what you're doing. You'll need to be
> +able to handle any conflicts in non-drm code while rebasing.
>   
> -Simply drop fixes that are already available in the new baseline.
> +Always ask for maintainer ack before rebasing. IRC ack is sufficient.
> +
> +Simply drop fixes that are already available in the new baseline. Close the
> +associated gitlab issue when removing commits.
>   
>   Force pushing a rebased topic/core-for-CI requires passing the ``--force``
>   parameter to git::
> @@ -225,11 +227,22 @@ judgement call.
>   Only add or remove commits if you really know what you're doing. When in doubt,
>   ask the maintainers.
>   
> +Always ask for maintainer/committer ack before adding/removing commits. IRC ack
> +is sufficient. Record the ``Acked-by:`` in commits being added.
> +
>   Apply new commits on top with regular push. The commit message needs to explain
>   why the patch has been applied to topic/core-for-CI. If it's a cherry-pick from
>   another subsystem, please reference the commit with ``git cherry-pick -x``
>   option. If it's a patch from another subsystem, please reference the patch on
>   the mailing list with ``Link:`` tag.
>   
> +New commits always need an associated gitlab issue for tracking purposes. The
> +goal is to have as few commits in topic/core-for-CI as possible, and we need to
> +be able to track the progress in making that happen. Reference the issue with
> +``References:`` tag. Add the ``core-for-CI`` label to the issue. (Note: Do not
> +use ``Closes:`` because the logic here is backwards; the issue is having the
> +commit in the branch in the first place.)
> +

In some cases we can have two situations depending on which team is 
quicker - 1) there may be a gitlab issue created already by the 
regression tracking team, 2) maintainer/committer may need to create a 
new issue. (Duplicates may arise as well, although I don't think that is 
a problem. The only important thing is that the core-for-CI commit gets 
tracked/justified.) So perhaps just clarify if the idea is for person 
adding a commit to search for bugs or just create a new issue?

But in general I think it is a good idea to record justification in 
gitlab. In practice we ended up with a number of "permanent" patches in 
there, which probably are not ever going away and those are probably the 
ones which are most important to document why they are there. Or at 
least no one is doing anything to upstream them or something. With 
gitlab tracking it is not guaranteed that would be better, but at least 
there would be some conversation history.

Regards,

Tvrtko

>   Instead of applying reverts, just remove the commit. This implies ``git rebase
> --i`` on the current baseline; see directions above.
> +-i`` on the current baseline; see directions above. Close the associated gitlab
> +issue when removing commits.