On Mon, 9 Jun 2025, Alejandro Vallejo wrote: > On Fri Jun 6, 2025 at 9:59 PM CEST, Stefano Stabellini wrote: > > On Fri, 6 Jun 2025, Alejandro Vallejo wrote: > >> On Fri Jun 6, 2025 at 10:59 AM CEST, Michal Orzel wrote: > >> > > >> > > >> > On 05/06/2025 21:48, Alejandro Vallejo wrote: > >> >> Part of an unpicking process to extract bootfdt contents independent of > >> >> bootinfo > >> >> to a separate file for x86 to take. > >> >> > >> >> Move functions required for early FDT parsing from device_tree.h and > >> >> arm's > >> >> setup.h onto bootfdt.h > >> >> > >> >> Declaration motion only. Not a functional change. > >> >> > >> >> Signed-off-by: Alejandro Vallejo <agarc...@amd.com> > >> >> --- > >> >> v2: > >> >> * Remove the u32 identifiers in the device_tree_get_u32() > >> >> implementation > >> > I don't understand the reasoning behind changing u32->uint32_t only for > >> > one > >> > function in this patch while leaving others unmodified. Also what about > >> > u64? > >> > Either don't change any or change all. > >> > >> Sure. Let's call the original u32->uint32_t change a misplaced mutation and > >> move on. The point is the motion, not these cleanups on top. > > > > Yes I agree. I know from past experience that Jan doesn't mind changes > > during code movements, but for me it is important that changes and code > > movement are separate. That is because I have almost automatic ways to > > check that code movement is correct if there are no changes. It saves me > > a lot of time during review. Then I can look at the individual changes > > separately. > > That's interesting. Could you please share the runes? That's one side of > review I still struggle with.
I use vim to generate two source files, one with the code corresponding to the - lines (with the - prefix) and one with the code corresponding to the + lines (without the + prefix). Then I diff the two files. This method is a bit crude but very effective and managed to spot issues this way more than once.