[Buildroot] [PATCH] fs/ubifs: factorize all the "depends on" into "if...endif" blocks

[Buildroot] [PATCH] fs/ubifs: factorize all the "depends on" into "if...endif" blocks

[Mark Jackson]

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(-)

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
-- 
1.7.9.5

[Buildroot] [PATCH] fs/ubifs: factorize all the "depends on" into "if...endif" blocks

[Yann E. MORIN]

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] fs/ubifs: factorize all the "depends on" into "if...endif" blocks

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH] fs/ubifs: factorize all the "depends on" into "if...endif" blocks

[Mark Jackson]

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.

[Buildroot] [PATCH] fs/ubifs: factorize all the "depends on" into "if...endif" blocks

[Yann E. MORIN]

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +0/33 662376056 | Software  Designer | \ / CAMPAIGN     |   ^                |
| --==< O_o >==-- '------------.-------:  X  AGAINST      |  /e\  There is no  |
| http://ymorin.is-a-geek.org/ | (*_*) | / \ HTML MAIL    |  """  conspiracy.  |
'------------------------------'-------'------------------'--------------------'