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

[Jani Nikula]

On Tue, 25 Oct 2022, Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com> wrote:
> 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?

I don't really care who creates it, as long as there's one. But anyone
tracking regressions might file an issue, and then close it because the
commit in core-for-CI "fixes" it. For this, the issue should be fixed by
removing the commit from core-for-CI. The "bug" is having the commit in
core-for-CI. So the two are kind of backwards. Maybe we should just file
a new bug always?

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

I look at most of the old ones, and I can't help thinking we've just
completely dropped the ball after merging the changes to
core-for-CI. And that's exactly why I want tracking, and I want the
commits upstreamed or simply dropped. We can't keep maintaining this
stuff ourselves.

BR,
Jani.


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

-- 
Jani Nikula, Intel Open Source Graphics Center