Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Yann E. MORIN <yann.morin.1998@free.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] utils/checkpackagelib: exclude two files from Config.in indentation check
Date: Wed, 20 Dec 2017 12:06:06 +0100	[thread overview]
Message-ID: <20171220110606.GA10605@scaer> (raw)
In-Reply-To: <20171219092852.4930a601@windsurf>

Thomas, All,

On 2017-12-19 09:28 +0100, Thomas Petazzoni spake thusly:
> On Mon, 18 Dec 2017 23:03:44 +0100, Yann E. MORIN wrote:
> > Can't we simply fix those two files to not have that unusual indentation
> > in the first place?
> > 
> > AFAICS, that special indentation is about the 'source' lines... Surely
> > that does not bring much to have them indented, and they could simply be
> > left-aligned...
> 
> package/Config.in is also indented in the same way, and there's a
> reason: when you do a diff, diff will tell you in which section the
> change is.

I know very well why package/Config.in is indented, I even Acked the
patch doing so, back in the day... ;-)

I'm just questioning if we could instead drop that indentation in the
two other files, because I believe it does not bring much for them.

> For example, if I do a change in the middle of "Audio and
> video applications", I get:
> 
> diff --git a/package/Config.in b/package/Config.in
> index bd39a374f0..dbcf53d1ce 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -31,7 +31,7 @@ menu "Audio and video applications"
>         source "package/mimic/Config.in"
>         source "package/miraclecast/Config.in"
>         source "package/mjpegtools/Config.in"
> -       source "package/modplugtools/Config.in"
> +       source "package/foomodplugtools/Config.in"
>         source "package/motion/Config.in"
>         source "package/mpd/Config.in"
>         source "package/mpd-mpc/Config.in"
> 
> See the @@ menu "Audio and video applications" ? You wouldn't get this
> without the indentation.

Thanks, I know how to read a diff... ;-)

But it does not always work, see:

diff --git a/package/Config.in b/package/Config.in
index bd39a374f0..83bb126cb8 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -312,6 +312,7 @@ endif
 	source "package/tekui/Config.in"
 	source "package/weston/Config.in"
 	source "package/x11r7/Config.in"
+	source "package/blabla/Config.in"
 
 comment "X applications"
 	depends on BR2_PACKAGE_XORG7
@@ -375,6 +376,7 @@ endmenu
 	source "package/a10disp/Config.in"
 	source "package/acpica/Config.in"
 	source "package/acpid/Config.in"
+	source "package/foo/Config.in"
 	source "package/acpitool/Config.in"
 	source "package/aer-inject/Config.in"
 	source "package/am335x-pru-package/Config.in"

> So the indentation does serve a purpose, and has been intentionally
> added, at least to package/Config.in. I think the same
> reason/motivation applies to package/kodi/Config.in and
> package/x11r7/Config.in.

Arguably, and that's what I'm sayging: at least for those last two
files, we could well drop the idnentation because it is not so useful
(although it works better for them than for package/Config.in).

But oh well...

Regards,
Yann E. MORIN.

> Best regards,
> 
> Thomas Petazzoni
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

  reply	other threads:[~2017-12-20 11:06 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-18  8:43 [Buildroot] [PATCH] utils/checkpackagelib: exclude two files from Config.in indentation check Thomas Petazzoni
2017-12-18 22:03 ` Yann E. MORIN
2017-12-19  1:57   ` Ricardo Martincoski
2017-12-19  5:33     ` Bernd Kuhls
2017-12-19  8:28   ` Thomas Petazzoni
2017-12-20 11:06     ` Yann E. MORIN [this message]
2017-12-19  1:56 ` Ricardo Martincoski
2017-12-19  8:36   ` Thomas Petazzoni
2017-12-19 23:32     ` Ricardo Martincoski
2017-12-20  8:10       ` Thomas Petazzoni
2018-03-22  3:20 ` Ricardo Martincoski
2018-03-22  8:18   ` Thomas Petazzoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171220110606.GA10605@scaer \
    --to=yann.morin.1998@free.fr \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox