From: Stephen Boyd <bebarino@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCHv2 1/3] format-patch: create patch filename in one function
Date: Sat, 21 Mar 2009 22:59:24 -0700 [thread overview]
Message-ID: <49C5D3BC.3030100@gmail.com> (raw)
In-Reply-To: <7vwsai86nw.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> The renaming is a good idea even without any change in the feature.
> Naming functions after what their result is used _for_ is never a good
> idea, and we should name them after what they do.
>
> Does it still make sense to pass "keep_subject" to the function? After
> all what it does is to retain "[PATCH.." prefix that is useless for the
> purpose of making each patch easily identifiable. Because people almost
> always use patch acceptance tools in non-keep mode to strip the
> "[PATCH..]" prefix when creating the commits these days anyway, it may
> make more sense to lose the parameter altogether and simplify the
> processing.
I originally removed "keep_subject" but I took it out because I couldn't
figure out what it was being used for and I thought it might be
important. When it's removed all the tests pass. It's not even used by
other code besides a few if cases regarding numbering logic so I think
the entire command line switch could be removed.
>
> I also wonder if it makes sense to move what this function does into a
> user format; especially the logic that sanitizes the oneline string into
> filename friendly one may be something Porcelains may want an access to
> from outside.
>
> IOW, you can introduce a new format specifier (say, "%f") to
> format_commit_message() and the implemention of get_patch_filename() would
> just prepare a strbuf and call format_commit_message() on it, no?
This sounds great! I'm new so I don't know where to look for something
like this.
next prev parent reply other threads:[~2009-03-22 6:01 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-03-22 4:32 [PATCHv2 0/3] format-patch --attach/--inline use filename instead of SHA1 Stephen Boyd
2009-03-22 4:32 ` [PATCHv2 1/3] format-patch: create patch filename in one function Stephen Boyd
2009-03-22 4:32 ` [PATCHv2 2/3] format-patch: --attach/inline uses filename instead of SHA1 Stephen Boyd
2009-03-22 4:32 ` [PATCHv2 3/3] format-patch: --numbered-files and --stdout aren't mutually exclusive Stephen Boyd
2009-03-22 5:44 ` Junio C Hamano
2009-03-22 5:36 ` [PATCHv2 2/3] format-patch: --attach/inline uses filename instead of SHA1 Junio C Hamano
2009-03-22 5:31 ` [PATCHv2 1/3] format-patch: create patch filename in one function Junio C Hamano
2009-03-22 5:59 ` Stephen Boyd [this message]
2009-03-22 6:53 ` Junio C Hamano
2009-03-22 6:56 ` Stephen Boyd
2009-03-22 8:07 ` Junio C Hamano
2009-03-23 2:14 ` [PATCHv3 0/6] format-patch --attach/--inline uses filename not SHA1 Stephen Boyd
2009-03-23 2:14 ` [PATCHv3 1/6] pretty.c: add %f format specifier to format_commit_message() Stephen Boyd
2009-03-23 2:14 ` [PATCHv3 2/6] format-patch: construct patch filename in one function Stephen Boyd
2009-03-23 2:14 ` [PATCHv3 3/6] format-patch: pass a commit to reopen_stdout() Stephen Boyd
2009-03-23 2:14 ` [PATCHv3 4/6] format-patch: move get_patch_filename() into log-tree Stephen Boyd
2009-03-23 2:14 ` [PATCHv3 5/6] format-patch: --attach/inline uses filename instead of SHA1 Stephen Boyd
2009-03-23 2:14 ` [PATCHv3 6/6] format-patch: --numbered-files and --stdout aren't mutually exclusive Stephen Boyd
2009-03-31 22:17 ` [PATCHv3 1/6] pretty.c: add %f format specifier to format_commit_message() René Scharfe
2009-03-31 23:24 ` [PATCH] format_sanitized_subject: Don't trim past initial length of strbuf Stephen Boyd
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=49C5D3BC.3030100@gmail.com \
--to=bebarino@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/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.