On Tue, Oct 25, 2022 at 04:37:59PM +0300, Jani Nikula wrote: > 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.) > >> +
good idea! > > > > 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? yeap, in the worst case we can always mark the duplication and point to the real issue. The important part is to have a path of information from the patch itself to the history, even if this end up in some extra issues.gitlab hops. > > > 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. Nice doc clean-up and updates! Reviewed-by: Rodrigo Vivi <[email protected]> for the series > > > > > > 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
