Got it, I'm learning along the way, and thanks for the reply!

--
Thanks,
Shaoxuan

On Sat, Feb 5, 2022 at 7:59 PM Eric Sunshine [off-list ref] wrote:

On Fri, Jan 28, 2022 at 4:51 AM Shaoxuan Yuan [off-list ref] wrote:

quoted

On Fri, Jan 28, 2022 at 4:34 PM Eric Sunshine [off-list ref] wrote:

quoted

In this case, the indentation of the entire body of the for-loop needs
to be fixed to use tabs rather than spaces, however, fixing all the
indentation problems together with the other problems can make for a
too-noisy patch for reviewers to really see what is going on. As such,
it may be better to make this a multi-patch series in which one patch
fixes indentation problems only.

quoted

As mentioned above, changing the body of the test from double- to
single-quoted string should (presumably) be okay since the body gets
eval'd and `$p` will be visible at the time of `eval`, however, mixing
this sort of change in with all the other changes being made makes it
hard for reviewers to spot such little details, let alone reason about
correctness. As such, switching of quote types is also probably the
sort of change which should be split out into its own patch. The same
comment applies to other changes following this one.

I think so. I was thinking fixing all the general styling issues in one
patch (since they are all style related), but now I realize that the general
style patch can be divided into sub-patches for clearer review experience.

And my question is, should I do this "multi-patch series" thing in the form of
"-v<n>" (all under this thread), e.g. "v2" or "v3"? Or I just submit
multiple patches separately (a new thread for each one)?

A multi-patch series as v2, v3, etc. would indeed be appropriate, as
you already figured out[1] before I got around to answering belatedly.

quoted

quoted

Overall, with the exception of the test title which needs to
interpolate `$p`, the rest of the changes are probably reasonable and
benign. It's important to understand that lengthy patches like this
full of mechanical changes tend to be quite taxing on reviewers, so
it's a good idea to help in any way you can to ease the review burden.
This can be done, for instance, by making only a single type of change
per patch (i.e. indentation fixes), or by limiting the scope or
breadth of the changes, which is especially important for this sort of

I'm not quite sure what this means, and I quote, "limiting the scope or
breadth of the changes". Are you suggesting, for example,
fixing fewer occurrences of tab indentation issue in a patch; or, for
example, limiting the
fix to a specific "test_expect_success" block, and do multiple patches
sequentially?

Sorry for being unclear. I just meant that as a microproject, it would
have worked equally well to pick a much smaller test script with style
problems (if you could find one) rather than a long script. After all,
the purpose of a microproject is to give you experience with the
submission-review process and to give reviewers and mentors an idea of
how you interact. It's the process which is important, in this case,
not the size of the submission.

Anyhow, it looks like Junio is happy with your v3 and will be merging
it down to "next", so it all worked out.

[1]: https://lore.kernel.org/git/20220202064300.3601-1-shaoxuan.yuan02@gmail.com/ (local)
[2]: https://lore.kernel.org/git/xmqqr18jnr2t.fsf@gitster.g/ (local)