Junio C Hamano [off-list ref] writes:
Christian Couder [off-list ref] writes:
quoted
quoted
Is this really an error? It may be a warning-worthy situation for a
user or a script to end up doing a no-op graft, e.g.
git replace --graft HEAD HEAD^
but I wonder if it is more convenient to signal an error (like this
patch does) or just ignore the request and return without adding the
replace ref.
As the user might expect that a new replace ref was created on success
(0 exit code), and as we should at least warn if we would create a
commit that is the same as an existing one,...
Why is it an event that needs a warning? I do not buy that "as we
should at least" at all.
Ehh, it came a bit differently from what I meant. Perhaps s/do not
buy/do not understand/ is closer to what I think---that is, it is
not like I with a strong conviction think you are wrong. It is more
like I do not understand why you think it needs a warning, meaing
you would need to explain it better.
If you say "make sure A's parent is B" and then you asked the same
thing again when there already is a replacement in place, that
should be a no-op. "Making sure A's parent is B" would be an
idempotent operation, no? Why not just make sure A's parent is
already B and report "Your wish has been granted" to the user?
Why would it be simpler for the user to get an error, inspect the
situation and realize that his wish has been granted after all?
On Thu, Jul 10, 2014 at 7:36 PM, Junio C Hamano [off-list ref] wrote:
Junio C Hamano [off-list ref] writes:
quoted
quoted
As the user might expect that a new replace ref was created on success
(0 exit code), and as we should at least warn if we would create a
commit that is the same as an existing one,...
Why is it an event that needs a warning? I do not buy that "as we
should at least" at all.
Here you ask why this event needs a warning...
Ehh, it came a bit differently from what I meant. Perhaps s/do not
buy/do not understand/ is closer to what I think---that is, it is
not like I with a strong conviction think you are wrong. It is more
like I do not understand why you think it needs a warning, meaing
you would need to explain it better.
quoted
If you say "make sure A's parent is B" and then you asked the same
thing again when there already is a replacement in place, that
should be a no-op.
(When there is already a replacement in place we error out in
replace_object_sha1() unless --force is used. What we are talking
about here is what happens if the replacement commit is the same as
the original commit.)
quoted
"Making sure A's parent is B" would be an
idempotent operation, no? Why not just make sure A's parent is
already B and report "Your wish has been granted" to the user?
... and here you say we should report "your wish has been granted"...
quoted
Why would it be simpler for the user to get an error, inspect the
situation and realize that his wish has been granted after all?
... but for me reporting to the user "your wish has been granted" and
warning (or errorring out) saying "the new commit would be the same as
the old one" are nearly the same thing.
So I wonder what exactly you are not happy with.
Is it the fact that I use the error() function, because it would
prefix the message with "fatal:" and that would be too scary?
Is it because with error() the exit code would not be 0?
Is it because the message "new commit is the same as the old one:
'%s'" is too scary or unnecessarily technical by itself?
Is it ok if I just print the message on stderr without "warning:" or
"fatal:" in front of it?
By the way, when I say "As ... and ..., I think it is just simpler to
error out in this case.", I mean that it is simpler from the
developer's point of view, not necessarily for the user.
Of course I am ok with improving things for the user even if it
requires more code/work, but it is difficult to properly do that if I
don't see how I could do it.
Thanks,
Christian.