On Tue, 25 Oct 2022, Tvrtko Ursulin <[email protected]> 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 <[email protected]> >> --- >> 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
