GNOME Bugzilla – Bug 750590
[review] lr/cli-add-properties: make it possible to specify properties upon connection addition
Last modified: 2015-07-02 07:39:01 UTC
The point is to atomically add a connection with (or without) desired properties, without a separate add and modify commands (since the addition of autoconnect=yes connection would trigger autoactivation). 83e825c cli: allow specifying arbitrary properties on "nmcli c add" 495aec5 cli: refactor: split connection property reader from do_connection_modify() 19d9db5 cli: trivial: move is_property_valid()
does this solve bug 740749?
LGTM and seems very useful.
Please refer to the related bz in the commit message: https://bugzilla.gnome.org/show_bug.cgi?id=698076 https://bugzilla.gnome.org/show_bug.cgi?id=740749 (and close them after merging this BZ).
locks good. Still needs bash-completion. Contrary to what we do for `nmcli connection modify` (which asks nmcli connection show for valid properties), just complete with a static list of options.
> cli: refactor: split connection property reader from do_connection_modify() While you're there, it looks like 'property_name' gets leaked each time through the while() loop. Maybe move the 'char *property_name' into the while loop and then use "gs_free char *property_name = NULL" to fix that up? Could also benefit from further gsystem-local-alloc fixups for 'strv' and 'local'. The rest LGTM.
(In reply to Thomas Haller from comment #4) > Still needs bash-completion. Contrary to what we do for `nmcli connection > modify` (which asks nmcli connection show for valid properties), just > complete with a static list of options. Done. Abused the editor. (In reply to Dan Williams from comment #5) > > cli: refactor: split connection property reader from do_connection_modify() > > While you're there, it looks like 'property_name' gets leaked each time > through the while() loop. Maybe move the 'char *property_name' into the > while loop and then use "gs_free char *property_name = NULL" to fix that up? Done. c68a1ba cli: trivial: move is_property_valid() b12c042 cli: refactor: split connection property reader from do_connection_modify() 3fb476d cli: allow specifying arbitrary properties on "nmcli c add" ab39eda cli: add bash completion for 'nmcli c add -- <property list>'
(In reply to Lubomir Rintel from comment #6) > (In reply to Thomas Haller from comment #4) > > Still needs bash-completion. Contrary to what we do for `nmcli connection > > modify` (which asks nmcli connection show for valid properties), just > > complete with a static list of options. > > Done. Abused the editor. Pushed a fixup! commit too. > (In reply to Dan Williams from comment #5) > > > cli: refactor: split connection property reader from do_connection_modify() > > > > While you're there, it looks like 'property_name' gets leaked each time > > through the while() loop. Maybe move the 'char *property_name' into the > > while loop and then use "gs_free char *property_name = NULL" to fix that up? > > Done. > > c68a1ba cli: trivial: move is_property_valid() > b12c042 cli: refactor: split connection property reader from > do_connection_modify() > 3fb476d cli: allow specifying arbitrary properties on "nmcli c add" > ab39eda cli: add bash completion for 'nmcli c add -- <property list>' LGTM
39ed60d merge,cli: branch 'lr/cli-add-properties' 81f1e3d cli: add bash completion for 'nmcli c add -- <property list>' b3e57cf cli: allow specifying arbitrary properties on "nmcli c add" 15149d9 cli: refactor: split connection property reader from do_connection_modify() df69bd1 cli: trivial: move is_property_valid()