[ 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 <sula...@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
