| Message ID | 11b296a55e9e450575c64ade1a2cb93a1886b9da.1630947142.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Gets rid of "if reflog exists, append to it regardless of config settings" | expand |
On Mon, Sep 06 2021, Han-Wen Nienhuys via GitGitGadget wrote: > From: Han-Wen Nienhuys <hanwen@google.com> > > Follow the reflog format more closely, so it can be used for comparing > reflogs in tests without using inspecting files under .git/logs/ > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > --- > t/helper/test-ref-store.c | 5 ++--- > t/t1405-main-ref-store.sh | 1 + > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c > index b314b81a45b..0fcad9b3812 100644 > --- a/t/helper/test-ref-store.c > +++ b/t/helper/test-ref-store.c > @@ -151,9 +151,8 @@ static int each_reflog(struct object_id *old_oid, struct object_id *new_oid, > const char *committer, timestamp_t timestamp, > int tz, const char *msg, void *cb_data) > { > - printf("%s %s %s %"PRItime" %d %s\n", > - oid_to_hex(old_oid), oid_to_hex(new_oid), > - committer, timestamp, tz, msg); > + printf("%s %s %s %" PRItime " %+05d\t%s\n", oid_to_hex(old_oid), > + oid_to_hex(new_oid), committer, timestamp, tz, msg); Nit: Would be a more readable diff if this wasn't a line-wrap-while-at-it change in addition to changing the format string. I.e. the last 4x parameters aren't changed, so leaving them on their own line & just changing the string & the two oid_to_hex()...
On Tue, Sep 7, 2021 at 12:35 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > - printf("%s %s %s %"PRItime" %d %s\n", > > - oid_to_hex(old_oid), oid_to_hex(new_oid), > > - committer, timestamp, tz, msg); > > + printf("%s %s %s %" PRItime " %+05d\t%s\n", oid_to_hex(old_oid), > > + oid_to_hex(new_oid), committer, timestamp, tz, msg); > > Nit: Would be a more readable diff if this wasn't a > line-wrap-while-at-it change in addition to changing the format string. > > I.e. the last 4x parameters aren't changed, so leaving them on their own > line & just changing the string & the two oid_to_hex()... This is clang-format's output, and the new code takes up less lines vertically. If you think this is really, really important, I can change it, but I think it's a better use of everyone's time to leave mechanical tasks (like formatting) to the machines.
On Tue, Sep 07 2021, Han-Wen Nienhuys wrote: > On Tue, Sep 7, 2021 at 12:35 AM Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> > - printf("%s %s %s %"PRItime" %d %s\n", >> > - oid_to_hex(old_oid), oid_to_hex(new_oid), >> > - committer, timestamp, tz, msg); >> > + printf("%s %s %s %" PRItime " %+05d\t%s\n", oid_to_hex(old_oid), >> > + oid_to_hex(new_oid), committer, timestamp, tz, msg); >> >> Nit: Would be a more readable diff if this wasn't a >> line-wrap-while-at-it change in addition to changing the format string. >> >> I.e. the last 4x parameters aren't changed, so leaving them on their own >> line & just changing the string & the two oid_to_hex()... > > This is clang-format's output, and the new code takes up less lines vertically. > > If you think this is really, really important, I can change it, but I > think it's a better use of everyone's time to leave mechanical tasks > (like formatting) to the machines. It's not "really, really important" or even "really important", just notes while reading the series. Patches are read N times, including during review. Since humans read formatting changes, it's in general are not something we can leave to machines. We're after all looking at the patch on-list, not diffing two versions of the generated machine code, or of clang-format's output. I don't think the intent of the clang-format target is to run it before patch submission, but to serve as input on suggested formatting changes. If on master you run: git ls-files '*.[ch]' -z | xargs -n 1 -0 clang-format -i You'll get: 631 files changed, 48246 insertions(+), 45061 deletions(-) Which I think speaks for itself in the likelyhood that "git-clang-format" is going to inject unnecessary churn into submitted patches. I think this case is quite small and not worth a re-roll.
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index b314b81a45b..0fcad9b3812 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -151,9 +151,8 @@ static int each_reflog(struct object_id *old_oid, struct object_id *new_oid, const char *committer, timestamp_t timestamp, int tz, const char *msg, void *cb_data) { - printf("%s %s %s %"PRItime" %d %s\n", - oid_to_hex(old_oid), oid_to_hex(new_oid), - committer, timestamp, tz, msg); + printf("%s %s %s %" PRItime " %+05d\t%s\n", oid_to_hex(old_oid), + oid_to_hex(new_oid), committer, timestamp, tz, msg); return 0; } diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh index a600bedf2cd..76b15458409 100755 --- a/t/t1405-main-ref-store.sh +++ b/t/t1405-main-ref-store.sh @@ -94,6 +94,7 @@ test_expect_success 'for_each_reflog_ent()' ' test_expect_success 'for_each_reflog_ent_reverse()' ' $RUN for-each-reflog-ent-reverse HEAD >actual && + head -n1 actual | grep recreate-main && tail -n1 actual | grep one '