On Mon, Mar 20, 2023 at 7:56 PM Gregory Stark (as CFM) <stark.cfm@gmail.com> wrote:
On Wed, 4 Jan 2023 at 10:05, Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote: > > Thanks, I have modified everything as suggested, except one point > > > Don't use variable format strings. They're hard to read and they > > probably defeat compile-time checks that the arguments match the > > format string. You could use a "*" field width instead, ie ... > > * Another thought is that the non-text formats tend to prize output > > consistency over readability, so maybe we should just always use 2 > > fractional digits there, rather than trying to minimize visible changes. > > In that, we need to adjust a lot of test case outputs.
> > * We need some doc adjustments, surely, to explain what the heck this > > means.
That sounds like three points :) But they seem like pretty good arguments to me and straightforward if a little tedious to adjust.
This patch was marked Returned with Feedback and then later Waiting on Author. And it hasn't had any updates since January. What is the state on this feedback? If it's already done we can set the patch to Ready for Commit and if not do you disagree with the proposed changes?
If there is a consensus to modify the test cases' output, I am willing to
make the necessary changes and adjust the patch accordingly. However,
if there is a preference to keep the output of certain test cases unchanged,
I can rebase and modify the patch accordingly to accommodate those preferences.
It's actually the kind of code cleanup changes I'm reluctant to bump a patch for. It's not like a committer can't make these kinds of changes when committing. But there are so many patches they're likely to just focus on a different patch when there are adjustments like this pending.