Thread (8 messages) flat view 8 messages, 2 authors, 2021-06-15

Re: [PATCH] xdiff: implement a zealous diff3

From: Felipe Contreras <hidden>
Date: 2021-06-15 10:24:11

Jeff King wrote:
On Mon, Jun 14, 2021 at 11:19:46PM -0500, Felipe Contreras wrote:
quoted
quoted
My point is that if you are going to repost a patch that has known
problems,
It was not known that it had problems.

That fact that person X said patch Y had a problem doesn't necessarily
mean that patch Y has a problem.

  1. The problem in the past might not apply in the present
  2. The problem X person had might be specific to his/her setup
  3. The problem might be due a combination of patches, not the patch
     itself

Plus many others.

A logical person sees evidence for what it is, and the only thing that
person X saying patch Y had a problem means, is that person X said patch
Y had a problem.
Wow.

For one thing, you could still relay the _report_ of a problem along
with the patch, which would be valuable information for reviewers.
Yes I could have, and knowing what I know now I wouldn't even have even
posted the patch (not without a proposed fix). Woulda, coulda, shoulda.

But that's not the point. The point is that I did not repost a patch with
known problems *today*. Nor did I know what kind of problems, or
how pervasive the issue was.

Presumably you had to try at least 2,500 merges to find *one* issue.

I ran all the tests for diff3 with zdiff3 and they passed without
problems.


Merging this patch would have:

 1. Not broken any tests
 2. Not changed any behavior for any user
 3. Not have caused any problem for the vast majority (> 99%) of
    people trying out zdiff3

So there was no carelessness here.

Moreover, I provied the patch at 9:30, at 10:42 you commented about the
segfault, and 16:24 I had the fix. On a Sunday.

If this is not caring, I don't know what is.

-- 
Felipe Contreras
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help