Hi! thanks for the review.
On Thu, Jan 24, 2013 at 6:54 PM, Iustin Pop <[email protected]> wrote: > On Thu, Jan 24, 2013 at 04:52:18PM +0100, Helga Velroyen wrote: > > This is the beginning of the implementation of network > > queries. This includes establishing all infrastructure > > to run the network queries and implement querying of > > some simpler fields and the node group listing. > > LGTM, some minor notes below: > > > +-- | Retrieves the network's mode and formats it human-readable, > > +-- also in case it is not available. > > +getMode :: PartialNicParams -> String > > +getMode nic_params = > > + case nicpModeP nic_params of > > + Just mode -> nICModeToRaw mode > > + Nothing -> "-" > > Just FYI, the above is idiomatically written: > > maybe "-" nICModeToRaw $ nicpModeP nic_params > > (similar to fromMaybe below, but with a transformation function on the > value inside the Just). > Thanks for the hint, will fix it. > > > +-- | Retrieves the network's link and formats it human-readable, also in > > +-- case it it not available. > > +getLink :: PartialNicParams -> String > > +getLink nic_params = fromMaybe "-" (nicpLinkP nic_params) > > > Hmm, both getMode and getLink are (I think) too generically named in > this file which deals with other stuff. Should we name them > getNicMode/getNicLink? > Good point. I will make the names more specific. Cheers, Helga --
