From: Junio C Hamano <gitster@pobox.com>
To: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Cc: Nadav Goldstein via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org,
Nadav Goldstein <nadav.goldstein96@gmail.com>
Subject: Re: [PATCH] Add 'preserve' subcommand to 'git stash'
Date: Sat, 17 Jun 2023 04:21:59 -0700 [thread overview]
Message-ID: <xmqqwn02qqp4.fsf@gitster.g> (raw)
In-Reply-To: ZI1xLwemOs9Vxorf@ugly
Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
> i may be totally wrong about it (because i don't understand the
> motivation behind this feature, either), but i think the _intent_ of
> nadav's patch is to merely expose the first half of "stash push" (the
> other half is the implicit "reset --hard"). it may not be a
> sufficiently good one, but there is clearly an analogy here.
I do agree that it would be reasonable to want to expose the first
half (the other half is "now the local mod got saved in a stash,
adjust the working tree and/or the index"), but then that means the
code should cover the various operating modes we have, and let the
users perform their first half, so that the second half (which by
the way needs to be exposed by another series later) can be used on
top of the result to emulate as if the combined two (i.e. "stash
save/push") have been run, for the feature to be complete, no?
Lack of the second half can be excused away with "let's do these one
step at a time", but the analogy fails to hold with an incomplete
coverage of even the first half, I am afraid.
But as you said, I think the lack of concrete "here is how this
feature is expected to be used and why it is useful because it
allows us to do X that we haven't been able to before" is the
largest first issue in the posted patch, as that leaves reviewers
guessing without feeling they "understand the motivation behind" the
feature. Such an understanding would help us to tell where to stop
(maybe in certain modes doing only the "first half" does not make
sense because the corresponding "second half" inherently does not
exist for some reason, in which case it is fine not to support such
a mode that is supported by "stash push").
next prev parent reply other threads:[~2023-06-17 11:22 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-16 11:00 [PATCH] Add 'preserve' subcommand to 'git stash' Nadav Goldstein via GitGitGadget
2023-06-16 16:42 ` Junio C Hamano
2023-06-16 20:03 ` Oswald Buddenhagen
2023-06-16 20:11 ` Junio C Hamano
2023-06-17 8:39 ` Oswald Buddenhagen
2023-06-17 11:21 ` Junio C Hamano [this message]
2023-06-18 9:05 ` Nadav Goldstein
2023-06-18 9:47 ` Oswald Buddenhagen
2023-06-18 10:57 ` Nadav Goldstein
2023-06-19 1:42 ` 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=xmqqwn02qqp4.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=nadav.goldstein96@gmail.com \
--cc=oswald.buddenhagen@gmx.de \
/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).