[ I committed 0001, then noticed I had a type in the subject line of
the commit message. Argh. ]
On Wed, Aug 7, 2024 at 9:41 AM Amul Sul <sulamul@gmail.com> wrote:
> With the patch, I am concerned that we won't be able to give an
> accurate progress report as before. We add all the file sizes in the
> backup manifest to the total_size without checking if they exist on
> disk. Therefore, sometimes the reported progress completion might not
> show 100% when we encounter files where m->bad or m->match == false at
> a later stage. However, I think this should be acceptable since there
> will be an error for the respective missing or bad file, and it can be
> understood that verification is complete even if the progress isn't
> 100% in that case. Thoughts/Comments?
When somebody says that something is a refactoring commit, my
assumption is that there should be no behavior change. If the behavior
is changing, it's not purely a refactoring, and it shouldn't be
labelled as a refactoring (or at least there should be a prominent
disclaimer identifying whatever behavior has changed, if a small
change was deemed acceptable and unavoidable).
I am very reluctant to accept a functional regression of the type that
you describe here (or the type that I postulated might occur, even if
I was wrong and it doesn't). The point here is that we're trying to
reuse the code, and I support that goal, because code reuse is good.
But it's not such a good thing that we should do it if it has negative
consequences. We should either figure out some other way of
refactoring it that doesn't have those negative side-effects, or we
should leave the existing code alone and have separate code for the
new stuff we want to do.
I do realize that the type of side effect you describe here is quite
minor. I could live with it if it were unavoidable. But I really don't
see why we can't avoid it.
--
Robert Haas
EDB: http://www.enterprisedb.com
