From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org
Subject: Re: [PATCH v2 0/9] Handle errors when setting configs
Date: Sat, 30 Jan 2016 17:03:42 +0100 [thread overview]
Message-ID: <20160130160342.GA790@pks-xps> (raw)
In-Reply-To: <xmqqsi1gi5af.fsf@gitster.mtv.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 1538 bytes --]
On Fri, Jan 29, 2016 at 10:55:52AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > The end result is roughly the same, but it's a lot less churn, and it's
> > more likely for new callers to get it right, because they have to go the
> > extra mile to ignore the error. I say "roughly" because it treats cases
> > we missed as "die", whereas yours leaves them as "ignore". I find it
> > highly unlikely that any of them actually _want_ the ignore behavior,
> > though.
>
> Yes, I like this approach better. It admittedly is more risky in
> that it would die if the conversion missed a case that should
> ignore, but I suspect that such a breakage would be found rather
> quickly (and the one that goes latent are the ones that do not
> matter in practice because people would not encounter them).
>
> > I'm just pondering, though. I don't find the "or_die" variant bad at
> > all, so if you really prefer it, I don't mind.
> >
> > Just to get a sense of what the reverse would look like, I worked up the
> > patch below (which compiles but does not link, as I did not actually
> > implement the "gently" form). Some error-checking call-sites are
> > converted to the "die" form, because that's essentially what happens
> > anyway (and I'd venture to say that the config code can provide a much
> > better error message).
>
> This variant certainly looks nicer to me, for the reasons give above.
Okay, thanks for your feedback. I'll create a new version next
week, then.
Patrick
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
prev parent reply other threads:[~2016-01-30 16:04 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-28 9:00 [PATCH v2 0/9] Handle errors when setting configs Patrick Steinhardt
2016-01-28 9:00 ` [PATCH v2 1/9] config: introduce set_or_die wrappers Patrick Steinhardt
2016-01-28 9:00 ` [PATCH v2 2/9] branch: return error code for install_branch_config Patrick Steinhardt
2016-01-28 9:00 ` [PATCH v2 3/9] remote: handle config errors in set_url Patrick Steinhardt
2016-01-28 9:00 ` [PATCH v2 4/9] clone: handle config errors in cmd_clone Patrick Steinhardt
2016-01-28 9:00 ` [PATCH v2 5/9] branch: handle config errors when unsetting upstream Patrick Steinhardt
2016-01-28 9:00 ` [PATCH v2 6/9] init-db: handle config errors Patrick Steinhardt
2016-01-28 9:00 ` [PATCH v2 7/9] sequencer: handle config errors when saving opts Patrick Steinhardt
2016-01-28 9:00 ` [PATCH v2 8/9] submodule--helper: handle config errors Patrick Steinhardt
2016-01-28 9:00 ` [PATCH v2 9/9] compat: die when unable to set core.precomposeunicode Patrick Steinhardt
2016-01-29 8:20 ` [PATCH v2 0/9] Handle errors when setting configs Jeff King
2016-01-29 18:55 ` Junio C Hamano
2016-01-30 16:03 ` Patrick Steinhardt [this message]
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=20160130160342.GA790@pks-xps \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--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 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.