All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.