On Fri, Dec 10, 2021 at 4:38 AM Fabian Stelzer <fs@xxxxxxxxxxxx> wrote:
> On 09.12.2021 00:11, Eric Sunshine wrote:
> >Fix broken &&-chains in `{...}` groups in order to reduce the number of
> >possible lurking bugs.
> > {
> >- echo "*.t filter=rot13"
> >+ echo "*.t filter=rot13" &&
> > echo "*.i ident"
> > } >.gitattributes &&
> > {
> >- echo "expanded-keywords ident"
> >+ echo "expanded-keywords ident" &&
> > echo "expanded-keywords-crlf ident text eol=crlf"
> > } >>.gitattributes &&
> >
>
> Wouldn't some of these be better off as heredocs as well?
> There are a couple more below. I personally don't much mind either way but
> since you changed quite a few in an earlier commit why not these?
It's been months since I made these changes, but I think there were at
least a couple reasons for not converting these to here-docs. First,
in these cases, there were only one or two missing `&&` per block. Had
I bulk converted them to here-docs, it would have made for a much more
noisy patch, which would have taxed reviewers more, and
reviewer-fatigue is a real concern when crafting a lengthy patch
series like this one. In the "here-doc conversion" patch, on the other
hand, many of those cases involved a significant number of missing
`&&`; often every line was missing `&&`. So, the changes in that patch
was going to be very noisy anyhow, whether I added missing `&&` or
converted to here-docs.
Second...
> > {
> > echoid insert 444 1 2 3 4 5 a b c d e &&
> >- echoid contains 44 441 440 444 4440 4444
> >+ echoid contains 44 441 440 444 4440 4444 &&
> > echo clear
> > } | test-tool oidtree >actual &&
... there are a number of cases like this which look like they could
easily be converted to here-doc, but in fact `echoid` is a function
call, so a here-doc wouldn't work. Also...
> > {
> > echo $(git rev-parse refs/tags/A) refs/tags/A &&
> >- echo $(git rev-parse refs/tags/A^0) "refs/tags/A^{}"
> >+ echo $(git rev-parse refs/tags/A^0) "refs/tags/A^{}" &&
> > echo $(git rev-parse refs/tags/C) refs/tags/C
> > } >expect &&
... this sort of thing could certainly become a here-doc because
$(...) will work in a here-doc, but when there is a preponderance of
this sort of `{ echo && ... }` block in the test script, it would feel
inconsistent to convert a few of them to here-docs.
