Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
> This is useful in certain SCMs like monotone, where each 'merge revision' has
> the changes of all the micro-branches merged. So it appears as duplicated commands.
The patch appears to add warning to when you try to 'D'elete something
that should not exist in the revision, whose moral equivalents are
implemented in the codepath to deal with 'R'enaming and 'C'opying an
non-existent path.
But instead of making it die(), it merely warns, and even worse, you are
demoting an existing die() in Rename/Copy codepath to mere warning
unconditionally. Why?
"This" that begins your proposed commit log message needs to be clarified,
but I am guessing that you are defending your change to demote existing
error check to die on inconsistent input to a mere warning. I do not find
it a particularly good defending argument. It sounds more like you are
papering over bugs in _one_ broken converter that produces and feeds an
incorrect input to fast-import, breaking a safety valve for everybody else.
> The delete command was ignoring the issue completely. The rename/copy commands
> where throwing a fatal exception.
Yes. I think this has to be two patch series, that:
(1) Adds the equivalent "you cannot delete what you do not have" die() in
the delete codepath; and
(2) Adds an option that demotes *all* the "don't touch (rename, modify,
copy or delete) what you do not have" die()s to warning().
provided if we can give a good rationale to the latter. Otherwise we
should just do (1) and forget about (2).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
