[bug#59761] [PATCH 0/2] Add u-boot-ts7970-q-2g-1000mhz-c.

[Maxim Cournoyer]

Hi Ricardo,

Ricardo Wurmus <rekado@elephly.net> writes:

> Hi Maxim,
>
> there seems to be some overlap between this and
> https://issues.guix.gnu.org/60224.

Yes, I ended up splitting my changes focusing on u-boot in #60224, which
should be reviewed before and blocking this change here, which is based
on it.

> Looking just at v4 I only have one
> comment.
>
> In your substitute* replacements it’s better not to use string-append.

Oh?  Why is this so?  There must be hundreds of string-append occurences
used in such place, so I'm curious.

> You can include real line breaks in a string and escape line breaks with
> \.  This is preferable to gluing strings together.

OK, I guess this is your rationale for the above comment (cleaner).

> For something as
> long as the replacements in this package consider using a patch file
> instead.  This has the added advantage of failing the build when the
> patch cannot be applied cleanly.

I agree that a patch would be most suitable here, especially that if
something breaks, if would likely be silent (unlikely to be caught at
build time).  I'll extract this as a patch.

> The rest looks good to me.

OK.  I'll await your comments on #60224, which is awaiting feedback
post-rework based on your earlier feedback.

PS: I had also missed that email; please keep me in CC in all your
replies :-).

-- 
Thanks,
Maxim