git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Keith Cascio <keith@CS.UCLA.EDU>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Junio C Hamano <gitster@pobox.com>,
	Pierre Habouzit <madcoder@madism.org>, Jeff King <peff@peff.net>,
	git@vger.kernel.org
Subject: Re: [PATCH] Use DIFF_XDL_SET/DIFF_OPT_SET instead of raw bit-masking
Date: Wed, 18 Feb 2009 14:59:29 -0800 (PST)	[thread overview]
Message-ID: <alpine.GSO.2.00.0902181437260.6181@kiwi.cs.ucla.edu> (raw)
In-Reply-To: <alpine.DEB.1.00.0902182219370.10279@pacific.mpi-cbg.de>

On Wed, 18 Feb 2009, Johannes Schindelin wrote:

> Okay, you asked for it.  I tried to be gentle.
> 
> I see _no_ value in your changes, and the diffstat as a _very_ real downside.
> 
> If the code would become clearer with your patch, I would not mind.  But I 
> find that the result is not more readable than the original.
> 
> As part of a parse-optification, I would not mind.  But before that, no.

Actually I appreciate your feedback, and the more direct the better.  The chief 
value I see in revising the code to accomplish these bit settings uniformly 
using well known macros is that later, if someone has good reason to extend the 
macro so it results in some new side effect, e.g. to update a dirty bit mask, 
the new behavior automatically cascades to every appropriate use out there in 
the code.  A macro is essentially a "code constant".  As with other constants, 
the benefit comes from defining it once and using it everywhere.  Is this effect 
not worth as much as I think it is?  Is there a hidden gotcha in my ideal?  Or 
does anyone else see value here?  Please speak up!

                                             -- Keith

  reply	other threads:[~2009-02-18 23:01 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-17  3:26 [PATCH] Use DIFF_XDL_SET/DIFF_OPT_SET instead of raw bit-masking Keith Cascio
2009-02-17 12:05 ` Johannes Schindelin
2009-02-17 17:33   ` Keith Cascio
2009-02-17 22:57     ` Johannes Schindelin
2009-02-18 20:52       ` Keith Cascio
2009-02-18 21:22         ` Johannes Schindelin
2009-02-18 22:59           ` Keith Cascio [this message]
2009-02-19  1:02             ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.GSO.2.00.0902181437260.6181@kiwi.cs.ucla.edu \
    --to=keith@cs.ucla.edu \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=madcoder@madism.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).