On Wed, 08 Feb 2012 12:03:58 -0000 [email protected] wrote: > Author: mfilka > Date: Wed Feb 8 13:03:57 2012 > New Revision: 67377 > > URL: http://svn.opensuse.org/viewcvs/yast?rev=67377&view=rev > Log: > Base server hostname combobox on existing values - bnc#547983
Hi, congratulate to your first patch. I review your commit and have just small notes: > > snipped > > Modified: trunk/nfs-client/src/ui.ycp > URL: > http://svn.opensuse.org/viewcvs/yast/trunk/nfs-client/src/ui.ycp?rev=67377&r1=67376&r2=67377&view=diff > ============================================================================== > --- trunk/nfs-client/src/ui.ycp (original) > +++ trunk/nfs-client/src/ui.ycp Wed Feb 8 13:03:57 2012 > @@ -58,6 +58,11 @@ > list<string> hosts = nil; > > /** > + * List of already defined nfs mount points Well, comment is not precise. If you want list of already defined nfs entries, it is exactly Nfs::nfs_entries. This variable store new state of nfs_entries ( so you have old one in Nfs and this variable store modified list by user and when user decide it is stored ). > + */ > + list< map <string,any> > nfs_entries = Nfs::nfs_entries; > + > + /** > * Let the user choose one of a list of items > * @param title selectionbox title > * @param items a list of items > @@ -178,6 +183,7 @@ > `Bottom (button) > ); > } > + > /** > * Ask user for an entry. > * @param fstab_ent $["spec": "file": "mntops":] or nil > @@ -213,6 +219,21 @@ > servers = [ proposed_server ]; > } > > + // append already defined servers - bug #547983 good to have such comment, it really helps when WTF raised :) > + foreach( map nfs_entry, nfs_entries, > + { > + term couple = SpecToServPath( nfs_entry[ "spec"]:""); > + string known_server = ""; > + > + known_server = couple[0]:""; It is confusing to have multiple lines without intention. simple: string known_server couple[0]:""; is enough in this case. Otherwise good commit. Josef > snipped -- To unsubscribe, e-mail: [email protected] To contact the owner, e-mail: [email protected]
