Received: from vps892.directvps.nl (ikke.info [178.21.113.177])
	by nld3-dev1.alpinelinux.org (Postfix) with ESMTPS id 319FE78107B
	for <~alpine/devel@lists.alpinelinux.org>; Tue, 28 Apr 2020 08:06:48 +0000 (UTC)
Received: by vps892.directvps.nl (Postfix, from userid 1008)
	id 8E75D4400E1; Tue, 28 Apr 2020 10:06:47 +0200 (CEST)
Date: Tue, 28 Apr 2020 10:06:47 +0200
From: Kevin Daudt <kdaudt@alpinelinux.org>
To: Rasmus Thomsen <oss@cogitri.dev>
Cc: ~alpine/devel@lists.alpinelinux.org
Subject: Re: Enforcing patch headers for patches
Message-ID: <20200428080647.GB1838755@alpha>
References: <4ac4d3aad3bf0f050070166b27a2344efe32e9b7.camel@cogitri.dev>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4ac4d3aad3bf0f050070166b27a2344efe32e9b7.camel@cogitri.dev>

On Mon, Apr 27, 2020 at 12:23:43PM +0200, Rasmus Thomsen wrote:
> Hello,
> > I'd like to propose that we start enforcing patch headers for new
> patches in aports, to make the flood of (downstream) patches more
> manageable. With a patch header a patch would look like this:
> 
> $ cat xfce-screenshooter-1.7.9-dsofix.patch
> Upstream: Yes
> Reason: Without this patch linking with bfd fails due to missing linker
> flags, as these libraries are implicitly required.
> Url: https://bugzilla.xfce.org/show_bug.cgi?id=7985
> --- xfce4-screenshooter-1.7.9.orig/Makefile.in	2010-02-07
> 14:45:15.000000000 +0100
> +++ xfce4-screenshooter-1.7.9/Makefile.in	2010-02-16
> 23:57:31.000000000 +0100
> @@ -282,7 +282,7 @@
>  INTLTOOL_PERL = @INTLTOOL_PERL@
>  INTLTOOL_UPDATE = @INTLTOOL_UPDATE@
>  LD = @LD@
> -LDFLAGS = @LDFLAGS@
> +LDFLAGS = @LDFLAGS@ -lm -lX11
>  LIBOBJS = @LIBOBJS@
>  LIBS = @LIBS@
>  LIBTOOL = @LIBTOOL@
> 
> Let me explain the purpose of the different fields:
> 
> * Upstream: This should indicate whether the patch is proposed
> upstream/already merged or if it's specific to us. Should be "Yes" if
> it has been proposed/merged already, or "No" with an explanation why it
> can't be upstreamed. This helps tracking the status of a patch
> upstream. Mustn't be omitted.
> 
> * Reason: This describes what the patch is for. That makes it easier
> for others (or sometimes yourself at a later point) to figure out what
> a patch actually does. This field may be omitted if the patch is
> trivial and has a descriptive file name, or if the patch already has a
> commit message in it (e.g. from git format-patch).
> 
> * Url: An URL to where upstream deals with the issue, e.g. a link to
> upstream's issue report in their bugtracker or a link to a merge
> request. This may only be omitted if Upstream is "No" (meaning the
> patch is specific to Alpine Linux). This has the nice consequence that
> all patches that aren't specific to Alpine Linux have to be submitted
> to upstream, or the issue the patch tries to solve at least has to be
> reported at upstream. This lessens the maintenance burden on us (since
> we won't have to maintain as many downstream patches then) and makes
> the patches available for other developers as well.
> 
> While these patch header seem like a lot of boilerplate up front, it
> really does pay off in the long run. It probably takes less than a
> minute to write a patch header for the submitter of the patch since all
> of the required info is already readily available to the submitter:
> They know what the patch is for, what the upstream status is etc. It
> might take hours for other people who find out what a patch is for and
> whether it has been proposed upstream already at a later point.
> 
> Regards,
> 
> Rasmus

I think this is a good idea as well, just a few points:

* How are we going to handle the existing patches?
* What if there is already a commit message present in the patch? Are we
  going to prepend the header before that?

Kevin