Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jeremy Rosen <jeremy.rosen@openwide.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH v5 1/1] set simple network setup via the system configuration submenu
Date: Tue, 9 Dec 2014 10:23:04 +0100 (CET)	[thread overview]
Message-ID: <297168490.29020177.1418116984239.JavaMail.root@openwide.fr> (raw)
In-Reply-To: <20141209100258.79254fea@free-electrons.com>



----- Mail original -----
> Dear J?r?my Rosen,
> 
> On Tue,  9 Dec 2014 09:48:44 +0100, J?r?my Rosen wrote:
> 
> > +check_configuration ()
> > +{
> > +	if [ -z "$BR2_SIMPLE_NETWORK_NONE" ] ; then
> 
> Can we try to avoid unnecessary indentation, and so things the other
> way around, i.e if BR2_SIMPLE_NETWORK_NONE is not empty, bail out
> from
> the function?

can do...

> 
> 	if [ -n "$BR2_SIMPLE_NETWORK_NONE" ] ; then
> 		return
> 	fi
> 
> > +		if [ -z "$BR2_SIMPLE_NETWORK_NAME" ] ; then
> > +			echo ERROR no name specified for first network interface
> > +			exit 1
> > +		fi
> > +		if [ "$BR2_SIMPLE_NETWORK_IPV4_MANUAL" ] ; then
> 
> No condition?
> 


That was discussed in a previous iteration and was considered ok at
the time, but I can add an explicit condition


> > +	if [ -z "$BR2_SIMPLE_NETWORK_NONE" ] ; then
> 
> Same comment here.
> 

ok

> > +		echo "auto $BR2_SIMPLE_NETWORK_NAME"
> > +		if [ "$BR2_SIMPLE_NETWORK_IPV4_DHCP" ] ; then
> 
> And here.
> 

ok

> That being said, generally, I find this quite complicated, and my
> preference would be to continue with what we have today, and simply
> let
> the user override things with a root filesystem overlay or a
> post-build
> script.
> 

Well, the point is to avoid having an overlay just to enable DHCP, which
I thought was the direction BR wanted to go... simple stuff can be 
configured directly from menuconfig...


I can simplify the thing by dropping neworkd support if that makes you 
feel better, that would avoid the whole mess of parsing both mask formatq

but previous reviews had me add it to have it supported.

is that a formal rejection, or should I send a v6 ?


Best regards

J?r?my


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

  reply	other threads:[~2014-12-09  9:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-09  8:48 [Buildroot] [PATCH v5 1/1] set simple network setup via the system configuration submenu Jérémy Rosen
2014-12-09  9:02 ` Thomas Petazzoni
2014-12-09  9:23   ` Jeremy Rosen [this message]
2014-12-09 23:04 ` 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=297168490.29020177.1418116984239.JavaMail.root@openwide.fr \
    --to=jeremy.rosen@openwide.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