| Message ID | 5177D061.6020905@newflow.co.uk |
|---|---|
| State | Superseded |
| Headers | show |
Mark, All, On Wed, Apr 24, 2013 at 01:30:25PM +0100, Mark Jackson wrote: > All the UBIFS options use "depends on BR2_TARGET_ROOTFS_UBIFS" but > we can simplify the config file by enclosing them in an "if..endif" > block. > > We also do the same for the "depends on BR2_TARGET_ROOTFS_UBI". A few comments below (mostly nitpicking, but hey! ;-) ) > Signed-off-by: Mark Jackson <mpfj@newflow.co.uk> > --- > fs/ubifs/Config.in | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/fs/ubifs/Config.in b/fs/ubifs/Config.in > index f17c7dc..c1d2ce7 100644 > --- a/fs/ubifs/Config.in > +++ b/fs/ubifs/Config.in > @@ -3,21 +3,20 @@ config BR2_TARGET_ROOTFS_UBIFS > help > Build a ubifs root filesystem > > +if BR2_TARGET_ROOTFS_UBIFS [--SNIP--] > +if BR2_TARGET_ROOTFS_UBI [--SNIP--] > +endif > + > +endif Comment the endif-s so we know at a glance to which if they belong to: +endif # BR2_TARGET_ROOTFS_UBI + +endif # BR2_TARGET_ROOTFS_UBIFS With that fixed: Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Regards, Yann E. MORIN.
Dear Yann E. MORIN, On Wed, 24 Apr 2013 23:26:20 +0200, Yann E. MORIN wrote: > Comment the endif-s so we know at a glance to which if they belong to: > > +endif # BR2_TARGET_ROOTFS_UBI > + > +endif # BR2_TARGET_ROOTFS_UBIFS > > With that fixed: > Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Agreed. Thomas
On 24/04/13 22:26, Yann E. MORIN wrote: > Mark, All, > > On Wed, Apr 24, 2013 at 01:30:25PM +0100, Mark Jackson wrote: >> All the UBIFS options use "depends on BR2_TARGET_ROOTFS_UBIFS" but >> we can simplify the config file by enclosing them in an "if..endif" >> block. >> >> We also do the same for the "depends on BR2_TARGET_ROOTFS_UBI". > > A few comments below (mostly nitpicking, but hey! ;-) ) > >> Signed-off-by: Mark Jackson <mpfj@newflow.co.uk> >> --- >> fs/ubifs/Config.in | 18 ++++++++---------- >> 1 file changed, 8 insertions(+), 10 deletions(-) >> >> diff --git a/fs/ubifs/Config.in b/fs/ubifs/Config.in >> index f17c7dc..c1d2ce7 100644 >> --- a/fs/ubifs/Config.in >> +++ b/fs/ubifs/Config.in >> @@ -3,21 +3,20 @@ config BR2_TARGET_ROOTFS_UBIFS >> help >> Build a ubifs root filesystem >> >> +if BR2_TARGET_ROOTFS_UBIFS > [--SNIP--] >> +if BR2_TARGET_ROOTFS_UBI > [--SNIP--] >> +endif >> + >> +endif > > Comment the endif-s so we know at a glance to which if they belong to: > > +endif # BR2_TARGET_ROOTFS_UBI > + > +endif # BR2_TARGET_ROOTFS_UBIFS I *very* nearly put those in anyway, but checked with a few other Config.in files and none of them had such closing comments. > With that fixed: > Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr> I'll post a v2. Cheers Mark J.
Mark, All, On Thursday 25 April 2013 09:49:52 Mark Jackson wrote: > On 24/04/13 22:26, Yann E. MORIN wrote: > > Comment the endif-s so we know at a glance to which if they belong to: > > > > +endif # BR2_TARGET_ROOTFS_UBI > > + > > +endif # BR2_TARGET_ROOTFS_UBIFS > > I *very* nearly put those in anyway, but checked with a few other Config.in files > and none of them had such closing comments. Yes, probably. Old code did not always follow this, especially when the enclosed code blocks are small, or were small but later grew (which is very often the case), and people going like "Oh I don't need to put a comment on this closing conditional, it's just a few lines above. Oh, but now I added three more lines. Oh, and now someone else added twenty more lines. And now there is a new nested conditional block. Oh, and now the entire conditional block no longer fits on a screen. Oh, now I'm lost...". ;-) Which we should just avoid in the first place. > > With that fixed: > > Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > I'll post a v2. Thank you! :-) Regards, Yann E. MORIN.
diff --git a/fs/ubifs/Config.in b/fs/ubifs/Config.in index f17c7dc..c1d2ce7 100644 --- a/fs/ubifs/Config.in +++ b/fs/ubifs/Config.in @@ -3,21 +3,20 @@ config BR2_TARGET_ROOTFS_UBIFS help Build a ubifs root filesystem +if BR2_TARGET_ROOTFS_UBIFS + config BR2_TARGET_ROOTFS_UBIFS_LEBSIZE hex "UBI logical erase block size" - depends on BR2_TARGET_ROOTFS_UBIFS default 0x1f800 config BR2_TARGET_ROOTFS_UBIFS_MINIOSIZE hex "UBI minimum I/O size" - depends on BR2_TARGET_ROOTFS_UBIFS default 0x800 help Some comment required here config BR2_TARGET_ROOTFS_UBIFS_MAXLEBCNT int "Maximum LEB count" - depends on BR2_TARGET_ROOTFS_UBIFS default 2048 help Some comment required here @@ -25,7 +24,6 @@ config BR2_TARGET_ROOTFS_UBIFS_MAXLEBCNT choice prompt "ubifs runtime compression" default BR2_TARGET_ROOTFS_UBIFS_RT_LZO - depends on BR2_TARGET_ROOTFS_UBIFS help Select which compression format to use at run-time within the ubifs file system. @@ -50,7 +48,6 @@ endchoice choice prompt "Compression method" default BR2_TARGET_ROOTFS_UBIFS_NONE - depends on BR2_TARGET_ROOTFS_UBIFS help Select which compression format to compress the final image into. @@ -79,19 +76,18 @@ endchoice config BR2_TARGET_ROOTFS_UBIFS_OPTS string "Additional mkfs.ubifs options" - depends on BR2_TARGET_ROOTFS_UBIFS help Any additional mkfs.ubifs options you may want to include. config BR2_TARGET_ROOTFS_UBI - depends on BR2_TARGET_ROOTFS_UBIFS bool "Embed into an UBI image" help Build an ubi image from the ubifs one (with ubinize). +if BR2_TARGET_ROOTFS_UBI + config BR2_TARGET_ROOTFS_UBI_PEBSIZE hex "UBI physical erase block size" - depends on BR2_TARGET_ROOTFS_UBI default 0x20000 help Tells ubinize the physical eraseblock size of the flash chip @@ -99,7 +95,6 @@ config BR2_TARGET_ROOTFS_UBI_PEBSIZE config BR2_TARGET_ROOTFS_UBI_SUBSIZE int "UBI sub-page size" - depends on BR2_TARGET_ROOTFS_UBI default 512 help Tells ubinize that the flash supports sub-pages and the sub-page @@ -107,6 +102,9 @@ config BR2_TARGET_ROOTFS_UBI_SUBSIZE config BR2_TARGET_ROOTFS_UBI_OPTS string "Additional ubinize options" - depends on BR2_TARGET_ROOTFS_UBI help Any additional ubinize options you may want to include. + +endif + +endif
All the UBIFS options use "depends on BR2_TARGET_ROOTFS_UBIFS" but we can simplify the config file by enclosing them in an "if..endif" block. We also do the same for the "depends on BR2_TARGET_ROOTFS_UBI". Signed-off-by: Mark Jackson <mpfj@newflow.co.uk> --- fs/ubifs/Config.in | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)