From 67682d4adf1c38a8cb672a3699040fea17ed8189 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Tue, 12 Feb 2013 10:56:51 +1600 Subject: [PATCH] Re: [PATCH] convert bitmap to unsigned char --- 97/c0fdd004f9642502bf1d77fbaccf2b1cf37e7b | 135 ++++++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 97/c0fdd004f9642502bf1d77fbaccf2b1cf37e7b diff --git a/97/c0fdd004f9642502bf1d77fbaccf2b1cf37e7b b/97/c0fdd004f9642502bf1d77fbaccf2b1cf37e7b new file mode 100644 index 000000000..01d38a5f3 --- /dev/null +++ b/97/c0fdd004f9642502bf1d77fbaccf2b1cf37e7b @@ -0,0 +1,135 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 01B8A431FBC + for ; Mon, 11 Feb 2013 10:54:13 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 0.01 +X-Spam-Level: +X-Spam-Status: No, score=0.01 tagged_above=-999 required=5 + tests=[T_MIME_NO_TEXT=0.01] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id B8uIba-sBZ6y for ; + Mon, 11 Feb 2013 10:54:12 -0800 (PST) +Received: from arlo.cworth.org (arlo.cworth.org [50.126.95.6]) + (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 1FC16431FAE + for ; Mon, 11 Feb 2013 10:54:12 -0800 (PST) +Received: from yoom.home.cworth.org (localhost [127.0.0.1]) + by arlo.cworth.org (Postfix) with ESMTP id 2D07F6DE0C03; + Mon, 11 Feb 2013 10:54:10 -0800 (PST) +Received: by yoom.home.cworth.org (Postfix, from userid 1000) + id 9B76E60C12; Tue, 12 Feb 2013 05:56:57 +1100 (EST) +From: Carl Worth +To: Robert Mast , notmuch@notmuchmail.org +Subject: Re: [PATCH] convert bitmap to unsigned char +In-Reply-To: <1360501997-4967-1-git-send-email-beheerder@tekenbeetziekten.nl> +References: <1359917491-17178-1-git-send-email-beheerder@tekenbeetziekten.nl> + <1360501997-4967-1-git-send-email-beheerder@tekenbeetziekten.nl> +User-Agent: Notmuch/0.13.1 (http://notmuchmail.org) Emacs/23.4.1 + (x86_64-pc-linux-gnu) +Date: Mon, 11 Feb 2013 10:56:51 -0800 +Message-ID: <87r4kmk7bg.fsf@yoom.home.cworth.org> +MIME-Version: 1.0 +Content-Type: multipart/signed; boundary="=-=-="; + micalg=pgp-sha1; protocol="application/pgp-signature" +Cc: Robert Mast +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Mon, 11 Feb 2013 18:54:13 -0000 + +--=-=-= +Content-Transfer-Encoding: quoted-printable + +Robert Mast writes: +> --- + +Hi Robert, + +It looks like the git exercise is proving useful. Keep it up! + +If you look through the git logs a bit you'll find that the overwhelming +convention is for a commit message to have a single-line summary +followed by a (potentially longer) expanded comment. + +The convention I like best is for the single-line summary to +efficiently describe everything about "what" the patch does. If it's +hard to fit this into a single line there's a good chance your patch +should be split up. With your commit message here, the one-line summary +is great, (and much better than in your first submission). + +The expanded comment should describe the "why" of the change. And here, +your commit message doesn't have anything. So I'm left wondering why +your commit exists. Does this save memory? Does this fix some type error +or expected alignment somewhere? etc. + +Please re-submit your patch with a little more explanation of the +motivation behind the patch. + +=2DCarl + +PS. I do know that you did have more in the commit message originally, +and Austin recommended removing "snide" issues, (doubts and +meta-questions about the patch---things that don't belong in the commit +history). + +Your previous commit message was: + + Reading it in detail I thought it allocated way too much memory and + didn't use the full size of the allocated unsigned ints for storing + bits. + + Am I right, and is this the right way to patch code to notmuch? + +I'm not actually looking at the code in context now, so I can't render a +correct commit message, but this attempted rewording should hopefully +give you the idea: + + Using char instead of int allows for simpler definitions of the + DOCIDSET macros so the code is easier to understand. + + --- + + Am I reading that correctly? Or is there also some space saving + happening here as well? Please re-submit a new patch with a commit + message that makes things clear one way or the other. + +Do you see how this "why" part of the commit message is ready to stand +alone in our commit history (assuming it's correct and +accepted). Meta-questions like "is this even a sane way of doing +things?" are great to ask, and very helpful for code reviewers, but best +placed after the "---" so that they don't become part of the commit +message. + +Thanks for playing with notmuch! + +=2DCarl + +=2D-=20 +carl.d.worth@intel.com + +--=-=-= +Content-Type: application/pgp-signature + +-----BEGIN PGP SIGNATURE----- +Version: GnuPG v1.4.12 (GNU/Linux) + +iEYEARECAAYFAlEZPvMACgkQ6JDdNq8qSWhuawCfZTraPY2wUSv62MTQl5qhHO99 +lo8An1xH+tz66hfppsPU7KtTq/YiSVsA +=fzpL +-----END PGP SIGNATURE----- +--=-=-=-- -- 2.26.2