[Libosinfo] [PATCH 04/12] loader: Load volume-size for ISO9660 medias

[Christophe Fergeau]

On Mon, Mar 16, 2015 at 02:32:46PM +0000, Zeeshan Ali (Khattak) wrote:
> On Mon, Mar 16, 2015 at 2:23 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > On Mon, Mar 16, 2015 at 01:44:08PM +0000, Zeeshan Ali (Khattak) wrote:
> >> On Mon, Mar 16, 2015 at 9:57 AM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> >> > On Sun, Mar 15, 2015 at 04:04:51PM +0000, Zeeshan Ali (Khattak) wrote:
> >> >> ---
> >> >>  osinfo/osinfo_loader.c | 4 +++-
> >> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c
> >> >> index 845eda3..4e8f942 100644
> >> >> --- a/osinfo/osinfo_loader.c
> >> >> +++ b/osinfo/osinfo_loader.c
> >> >> @@ -999,7 +999,9 @@ static OsinfoMedia *osinfo_loader_media(OsinfoLoader *loader,
> >> >>               strcmp((const gchar *)nodes[i]->name,
> >> >>                      OSINFO_MEDIA_PROP_APPLICATION_ID) != 0 &&
> >> >>               strcmp((const gchar *)nodes[i]->name,
> >> >> -                    OSINFO_MEDIA_PROP_LANG) != 0))
> >> >> +                    OSINFO_MEDIA_PROP_LANG) != 0 &&
> >> >> +             strcmp((const gchar *)nodes[i]->name,
> >> >> +                    OSINFO_MEDIA_PROP_VOLUME_SIZE) != 0))
> >> >>              continue;
> >> >>
> >> >>          if (strcmp((const gchar *)nodes[i]->name,
> >> >
> >> > I'd squash the first patch (schema change) here as this is when we
> >> > change the XML parsing to handle the new attribute.
> >>
> >> Sure but then we'd need to add the API first and it seems weird for me
> >> to do it in that sequence.
> >
> > I don't understand your answer :(
> > 01/12 adds the new XML attributes to libosinfo XML schema, 04/12 parses
> > this new XML attribute, and I don't think 02/12 nor 03/12 needs this XML
> > attribute, so 01/12 can be removed and squashed into 04/12 without
> > moving any commit adding new API.
> > What API/what sequence are you worried about?
> 
> 02/12 adds the API to handle this attribute and I think it would be
> weird to introduce the API before adding the underlying XML.

I don't think this is weird, you add the needed API to the OsinfoMedia
class, and then you fill this data with what is read from XML files.
This API is used in the test cases to fill this data from other sources
than the XML files.
When I look at this, it's weird to have something in the XML schema, and
then only add support for parsing it later.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libosinfo/attachments/20150316/0397e856/attachment.sig>