On Sun, May 22, 2011 at 4:43 PM, Dave Reisner <d@falconindy.com> wrote:
Provide large warnings when net-tools functionality is used. Add documentation in rc.conf for the new iproute2 based config.
Functionality looks great (though untested). Nitpicks below.
diff --git a/PKGBUILD b/PKGBUILD index c5f2acd..5c9764c 100644 --- a/PKGBUILD +++ b/PKGBUILD @@ -9,9 +9,10 @@ groups=('base') conflicts=('initscripts') provides=('initscripts=9999') backup=(etc/inittab etc/rc.conf etc/rc.local etc/rc.local.shutdown) -depends=('glibc' 'bash' 'grep' 'coreutils' 'udev>=139-1' - 'net-tools' 'ncurses' 'kbd' 'findutils' 'sysvinit') -optdepends=('bridge-utils: Network bridging support' +depends=('glibc' 'bash' 'grep' 'coreutils' 'udev>=139-1' 'iproute2' + 'ncurses' 'kbd' 'findutils' 'sysvinit') +optdepends=('net-tools: legacy networking support' + 'bridge-utils: Network bridging support' 'dhcpcd: DHCP network configuration' 'wireless_tools: Wireless networking')
Not important, but the above hunk could easily have been made +/- 4 rather than +/- 7 (to make it easier to review :) ).
diff --git a/network b/network index 9cd6109..7350149 100755 --- a/network +++ b/network @@ -7,6 +7,49 @@ for s in wireless bonding bridges dhcpcd; do [[ -f /etc/conf.d/$s ]] && . "/etc/conf.d/$s" done
+# helper function to determine if legacy network support is needed +need_legacy() { + if [[ -z $interface ]]; then + return 0 # need legacy + fi + + return 1 # enough present for iproute2 support +}
Isn't the logic here inverted: 1=false and 0=true? Maybe just the name needs to change?
+deprecated() { + printf "${C_FAIL}Warning:${C_CLEAR} This functionality is deprecated.\n" + printf " Please refer to /etc/rc.conf on how to define a single wired\n" + printf " connection, or use a utility such as netcfg.\n" +}
I guess the people who will see this are the ones who have not updated their rc.conf's and hence might not have the new comments either, so maybe it would be better to put a reference to the wiki, and add the explanation there too? Just a suggestion. Cheers, Tom