[virt-tools-list] [libosinfo PATCHv2 3/9] Add osinfo_db_identify_media

[Christophe Fergeau]

On Wed, Dec 12, 2012 at 04:05:57AM +0200, Zeeshan Ali (Khattak) wrote:
> On Tue, Dec 11, 2012 at 10:17 PM, Christophe Fergeau
> <cfergeau at redhat.com> wrote:
> > +/**
> > + * osinfo_db_identify_media:
> > + * @db: a #OsinfoDb database
> > + * @media: the installation media
> > + * data
> > + *
> > + * Try to match the @media created using osinfo_media_create_from_location()
> > + * with a media description from @db. If found, @media will be filled with
> > + * the corresponding information stored in @db.
> 
> If we are going to keep media identifying API separate from media
> creation, I suggest we don't make it sound like media must be created
> using osinfo_media_create_from_location(). As you pointed out, we
> could easily have other constructors in future. When that happens, its
> very likely that we'll not remember to update this doc comment.
> 
> > In particular, after a call
> > + * to osinfo_media_create_from_location(), if the media could be
> > + * identified, its OsinfoMedia::os property will be set.
> 
> s/osinfo_media_create_from_location/osinfo_db_identify_media/ ? IMHO
> the ", after a call to osinfo_media_create_from_location()," part is
> quite redundant in this context.

I've fixed the 2 issues below.

> 
> > + * Returns: (transfer none): TRUE if @media was found in @db, FALSE
> 
> Not really an issue but for future ref: Transfer annotations are
> redundant for basic C/glib types.

Ah, probably a left-over from earlier iterations when this used to return
a GObject, I'll remove it as this indeed does not make a lot of sense here.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20121212/438cdd66/attachment.sig>