All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Parkins <andyparkins@gmail.com>
To: git@vger.kernel.org
Subject: Re: [PATCH 2/4] Rename remote_only to display_mode
Date: Fri, 3 Nov 2006 13:00:25 +0100	[thread overview]
Message-ID: <200611031200.27275.andyparkins@gmail.com> (raw)
In-Reply-To: <454B1F3B.1020603@op5.se>

On Friday 2006 November 03 10:51, Andreas Ericsson wrote:

> If you *need* to change something, change it. If you *want* to change
> something just because it's not written the way you would write it, back
> away. If you think some interface you're using needs clearing up
> (codewise or with extra comments), send a separate patch for that so the
> actual feature/bugfix you're sending in doesn't drown in cosmetic
> changes to the interfaces the patch uses/touches.

Thank you for the excellent advice.  What then would you suggest in the case 
in point?  I made as minimal a change as I could make; but that left the code 
a little bit bitty - I had press-ganged a variable into taking on another 
function and was using numeric literals that should really have been given 
meaning with #define?

My question is perhaps different from simply git-etiquette; it's should I 
prefer my patches to be minimal or neat?  If there is a more appropriate way 
of doing something should I do it or should I favour minimalism?

I've actually rewritten it now as per Junio's request, and while I'm happier 
with the code, it was much bigger change, that didn't really lend itself to 
being broken into smaller patches as did my first attempt.

I guess in the end it's a judgement call and the best thing to do is post it 
and see who shoots it down :-)


Andy

-- 
Dr Andy Parkins, M Eng (hons), MIEE

  parent reply	other threads:[~2006-11-03 12:00 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <bec6ab7849e3fcacac23cca44a0ba93282af5fca.1162465753.git.andyparkins@gmail.com>
2006-11-02 11:11 ` [PATCH 2/4] Rename remote_only to display_mode Andy Parkins
2006-11-03  2:40   ` Junio C Hamano
2006-11-03  8:40     ` Andy Parkins
2006-11-03 10:51       ` Andreas Ericsson
2006-11-03 11:51         ` Junio C Hamano
2006-11-03 12:37           ` Andy Parkins
2006-11-03 12:00         ` Andy Parkins [this message]
2006-11-03 13:23           ` Andreas Ericsson
2006-11-03 10:52     ` [PATCH] Add support to git-branch to show local and remote branches Andy Parkins
2006-11-03 12:06       ` [PATCH] Colourise git-branch output Andy Parkins
2006-11-03 19:25         ` Junio C Hamano
2006-11-03 23:05           ` Alex Riesen
2006-11-03 12:08       ` [PATCH] Add support to git-branch to show local and remote branches Junio C Hamano
2006-11-03 12:40         ` Andy Parkins
2006-11-03 19:19           ` Junio C Hamano
2006-11-02 11:11 ` [PATCH 3/4] Default to displaying /all/ non-tag refs, not just locals Andy Parkins
2006-11-03  2:40   ` Junio C Hamano
2006-11-03  8:47     ` Andy Parkins
2006-11-03 10:55       ` Andreas Ericsson
2006-11-02 11:11 ` [PATCH 4/4] Show the branch type after the branch name for remotes Andy Parkins
2006-11-03  2:40   ` Junio C Hamano
2006-11-03  8:33     ` Andy Parkins
2006-11-03 20:08       ` Junio C Hamano

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=200611031200.27275.andyparkins@gmail.com \
    --to=andyparkins@gmail.com \
    --cc=git@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.