Today on my SRU shift I spent some effort trying to see past quilt diff noise. Most of it seemed unnecessary and was only present because of gratuitous refreshes.
I'm not sure we have best practices are documented anywhere. I'd like to define some guidelines that make reviews easier. I think many people stick to these already. Can we agree that they're a good thing and try to get more people to do them? Summary 1. Use dep3 headers (https://dep-team.pages.debian.net/deps/dep3/) 2. Use "-p ab --no-timestamps --no-index" or equivalent (as documented at https://wiki.debian.org/UsingQuilt#Environment_variables). 3. Try not to update or refresh any quilt patch unless specifically required (ie. a patch doesn't apply, or applies with fuzz). Exception: do update dep3 headers. Details and rationale: 1. Use dep3 headers (https://dep-team.pages.debian.net/deps/dep3/) This makes it easy for reviewers to correlate the patch against upstream, find any related upstream commentary or subsequent related commits, etc. 2. Use "-p ab --no-timestamps --no-index" or equivalent (as documented at https://wiki.debian.org/UsingQuilt#Environment_variables). This reduces diff noise in the future if an update becomes required. Exception: if taking the patch from upstream and it applies as-is, then using the upstream form of the patch reduces the initial diff further, so that's preferable. So this combines with 3: use these settings when a refresh is required or generating the patch from scratch, but don't refresh to apply the settings to reduce noise unless actually required. 3. Try not to update or refresh any quilt patch unless specifically required (ie. a patch doesn't apply, or applies with fuzz). Exception: do update dep3 headers. This reduces the size of any diff taken against any other version of the quilt patch. Hopefully to zero. If one patch needs refreshing, refresh just that one rather than all of them. If a patch applies with offset, that's not a reason to refresh it. Example 1: you can append ".patch" to an upstream Github URL, download that to debian/patches/, rename and add dep3 headers but without modifying the patch itself. Then a reviewer can look at the dep3 header, identify that the origin was GitHub, diff against that same URL, and easily confirm that the patch hasn't been modified. Example 2: if I'm reviewing an SRU to multiple releases and the quilt patches are exactly identical, then I only have to review the patch itself once[1]. But if you've unnecessarily refreshed the patch on each upload, now I have a bunch of noise I have to review to verify that there is no functional change introduced. Does this sound reasonable to everyone to follow in general, such that we can document these as guidelines somewhere? I wouldn't expect them to be enforced as a hard rule, but once documented I also think it'd be reasonable for any reviewer to choose to push back where non-adherence is causing an actual problem. Anything to change, or anything to add? Thanks, Robie [1] The context outside the patch itself might be different and I do have to consider that too, but if I know the patches are identical, that is also made easier.
signature.asc
Description: PGP signature
-- ubuntu-devel mailing list ubuntu-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel
