[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Consistent patch style

On 05.09.24 22:17, Blair Noctis wrote:

Team,

This is somewhat nitpicking, but it's annoying to see different patch styles
clutter commits back and forth, sometimes making it hard to capture the actual
changes. Thus I suggest we discuss and agree on a team level patch style
everyone should follow.

There are a few things to consider:

1. Index line, like this:

Index a/foo
===================================================================
--- a/foo
+++ b/foo

This is largely redundant with the ---/+++ lines. I suggest we disable it.

To disable it, add QUILT_NO_DIFF_INDEX=1 in your ~/.quiltrc.

2. Timestamps, like this:

--- a/Cargo.toml    2024-09-05 19:58:55.244407500 +0000
+++ a/Cargo.toml    2024-09-05 19:58:55.244407500 +0000

I'm not sure if it carries more information than Last-Update field from DEP-3.

To disable it, add QUILT_NO_DIFF_TIMESTAMPS=1 in ~/.quiltrc.

3. Context radius

The default is 3, i.e. 3 lines each before and after the patched line. I argue
that sometimes reducing it is better for clarity and reduces the chance to fail
to apply on future updates.

Context radius can be specified as -Un for n lines. This applies to both quilt
and diff(1), for example, `quilt refresh -U1`.

For patching [dependencies] items in a crates.io normalized Cargo.toml (what we
face in debcargo-conf), -U1 is usually enough:

	--- a/Cargo.toml
	+++ b/Cargo.toml
	@@ -41,6 +41,6 @@
	 [dependencies.crossbeam]
	-version = "0.7"
	+version = "0.8"
	

and

	@@ -37,4 +38,4 @@
	
	-[dev-dependencies.bencher]
	-version = "0.1"
	+#[dev-dependencies.bencher]
	+#version = "0.1"
	

For patching features in Cargo.toml, and dependencies in Cargo.toml not
normalized, even -U0 is probably enough:

	--- a/Cargo.toml
	+++ b/Cargo.toml
	@@ -47 +47 @@
	-time = { version = "0.3.35", default-features = false, features =
["formatting", "local-offset", "macros"] }
	+time = { version = "0.3.30", default-features = false, features =
["formatting", "local-offset", "macros"] }

For other situations, use your discretion, but 3 lines are usually just overkill
IMO.

4. More?

==

Above is just my personal opinion put up for debate. Please don't hesitate to
provide your counter arguments ;)


+1 for a consistent patch style. I have this in my .quiltrc:


QUILT_PATCHES=debian/patches
QUILT_NO_DIFF_INDEX=1
QUILT_NO_DIFF_TIMESTAMPS=1
QUILT_REFRESH_ARGS="-p ab"
QUILT_DIFF_ARGS="--color=auto" # If you want some color when using `quilt diff`. 
QUILT_PATCH_OPTS="--reject-format=unified"
QUILT_COLORS="diff_hdr=1;32:diff_add=1;34:diff_rem=1;31:diff_hunk=1;33:diff_ctx=35:diff_cctx=33"

This produces already an output similar to what you proposed here (I think).

best,


werdahias

Attachment: OpenPGP_0xECBEDBB607B9B2BE.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature

Reply to: