On Feb 7, 2011, at 12:40 PM, Ben Pfaff wrote: > On Mon, Feb 07, 2011 at 11:48:37AM -0800, Justin Pettit wrote: >> Store the OVS version in OVSDB. Additionally, if the "lsb_release" >> command is available, store information about the system type and >> version. >> >> Bug #4576 > > This looks weird: > if [ `which lsb_release` ]; then > > I guess it works, since "which" prints nothing if it can't find the > program, and it will print one word beginning with / otherwise, and so > the 'test' program should do the right thing. But the most common idiom > is something more like > if (lsb_release --id) >/dev/null 2>&1; then > > If you really want "test" then could you make it more explicit: > if [ -n "`which lsb_release`" ]; then > > Looks OK otherwise, thank you.
I went with your first suggestion and pushed. Thanks for the review. --Justin _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
