Hi Maxime, On 07/09/2026 10:22, Maxime Chevallier wrote: > Hi, > > On 9/6/26 17:00, [email protected] wrote: >> Thank you for your contribution! Sashiko AI review found 3 potential >> issue(s) to consider: >> >> Critical: 0 · High: 0 · Medium: 2 · Low: 1 > > [...] > >> [Severity: Medium] >> Should the new include come before Makefile.arch rather than after it? > > This looks like a legit point > > [...] > >> [Severity: Medium] >> The commit message says the change accounts for "the cross-compiling options >> such as CROSS_COMPILE or the LLVM-specific variables". Does this also cover >> the documented LLVM form described in Documentation/kbuild/llvm.rst, that is >> make LLVM=1 ARCH=<arch> with no CROSS_COMPILE? > > I'll update the commit log to document the limitation > > [...] > >> [Severity: Low] >> This isn't a bug, but now that Makefile.include is pulled in here, the >> existing Q assignment later in this same file disagrees with it: > > Also legit as well. > > So, I'll swap the Makefile.arch and Makefile.include inclusions, document in > the commit log that we don't deal with LLVM + no CROSS_COMPILE, and drop the > > Q = @ > > line in ynltool/Makefile. > > Matthieu, are you OK with me keeping your review tag with the above changes, > or do I drop it and resend for you to re-review ?
Thank you for asking. I agree with the above changes, (and TBH, I didn't check for such issues in my review :) ), so you can keep my RvB tag. Cheers, Matt
