On Jan 31, 2011, at 2:31 PM, Scott Wood wrote: > On Mon, 31 Jan 2011 21:22:04 +0100 > Wolfgang Denk <w...@denx.de> wrote: > >> Dear Scott Wood, >> >> In message <20110131141332.5a4a2...@udp111988uds.am.freescale.net> you wrote: >>> >>>> I think these patches are incorrectly split. >>> >>> I think the intent was to split the arch-neutral stuff from the 85xx >>> stuff from the board stuff -- you'd rather they be all bunched together? >> >> No, of course not all together. >> >>>> This patch adds stuff to the Makefile, which would result in >>>> errors if used, as the referenced directories don't exist yet. >>> >>> Lots of patches add features, disabled by default, that require CPU or >>> board code to provide things, that would cause errors if the feature >>> were enabled on a board otherwise. >> >> But here nothing is disabled. It's added to the top level Makefile. >> It's dead code if unused, and causes errors if used. WHy not add the >> tpl target when you actually add the tpl code? >> >>> I don't think it's even possible to add an empty directory with git. >> >> True. Butt that would not fix anythign, it would still not work. >> >>>> [PATCH 2/6] powerpc/85xx: add TPL support >>>> >>>> This patch creates unused files, like >>>> arch/powerpc/cpu/mpc85xx/u-boot-tpl.lds >>> >>> It gets used in later in the patchset, when a board with tpl is added. >> >> Then this is where that file belongs to. > > I'm confused. You say "of course not all together", but the first one > you say to include with the second, and the second you say to include > with the third. > > If you're suggesting keeping them mostly separate, but just moving some > bits into the subsequent patch, that makes no sense to me. They > logically belong where they are -- e.g. > arch/powerpc/cpu/mpc85xx/u-boot-tpl.lds is part of 85xx TPL support, it > is not p1021mds-specific. And every bit of the first two patches is > technically dead until a board is added that uses it. > > Has your aversion to "dead" code grown so strong it can't exist even in > a transitory state between members of a patchset, even when necessary > to avoid mixing users of a facility with the facility itself in the > same patch? I think that would do significant harm to reviewability. > > -Scott
I'm in agreement with Scott on this. I believe we've taken this a bit too far about "dead code". It should be reasonable in a patch series to have code that will be used in a subsequent patch. - k _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
