[Libosinfo] [libosinfo][PATCH 5/5] media: Add 'network-installer' parameter

[Fabiano Fidêncio]

On Fri, Jun 8, 2018 at 11:00 AM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> On Thu, Jun 07, 2018 at 10:50:40PM +0200, Fabiano Fidêncio wrote:
>> Christophe,
>>
>> On Tue, Jun 5, 2018 at 11:12 AM, Christophe Fergeau <cfergeau at redhat.com> wrote:
>> >
>> >
>> > On Sun, Jun 03, 2018 at 06:44:32PM +0200, Fabiano Fidêncio wrote:
>> >> Apps can take advantage of this parameter in order to find out (and
>> >> correctly display to their users) whether network is needed during
>> >> the installation process.
>> >>
>> >> Signed-off-by: Fabiano Fidêncio <fabiano at fidencio.org>
>> >> ---
>> >>  osinfo/libosinfo.syms |  5 +++++
>> >>  osinfo/osinfo_media.c | 27 +++++++++++++++++++++++++++
>> >>  osinfo/osinfo_media.h |  2 ++
>> >>  3 files changed, 34 insertions(+)
>> >>
>> >> diff --git a/osinfo/libosinfo.syms b/osinfo/libosinfo.syms
>> >> index 64c6f79..3bad468 100644
>> >> --- a/osinfo/libosinfo.syms
>> >> +++ b/osinfo/libosinfo.syms
>> >> @@ -525,6 +525,11 @@ LIBOSINFO_0.2.13 {
>> >>      osinfo_media_get_eject_after_install;
>> >>  } LIBOSINFO_0.2.12;
>> >>
>> >> +LIBOSINFO_0.2.14 {
>> >> +    global:
>> >> +    osinfo_media_get_network_installer;
>> >
>> > I'd use the same indentation as was done before 0.2.13
>> >
>> >> +} LIBOSINFO_0.2.13;
>> >> +
>> >>  /* Symbols in next release...
>> >
>> >
>> > Acked-by: Christophe Fergeau <cfergeau at redhat.com>
>> >
>>
>> Apart from your comments, I've noticed that this fixup is needed:
>>
>
> Ah yeah definitely, dunno how I missed these :-/

And we missed a few more things :-)
I'll submit a v2.

For now, self-nack to this series.

>
>> diff --git a/osinfo/osinfo_media.c b/osinfo/osinfo_media.c
>> index 740ddcb..fb95d77 100644
>> --- a/osinfo/osinfo_media.c
>> +++ b/osinfo/osinfo_media.c
>> @@ -157,7 +157,8 @@ enum {
>>      PROP_OS,
>>      PROP_LANGUAGES,
>>      PROP_VOLUME_SIZE,
>> -    PROP_EJECT_AFTER_INSTALL
>> +    PROP_EJECT_AFTER_INSTALL,
>> +    PROP_NETWORK_INSTALLER
>>  };
>>
>>  static void
>> @@ -344,6 +345,13 @@ osinfo_media_set_property(GObject      *object,
>>                                          g_value_get_boolean(value));
>>
>>          break;
>> +
>> +    case PROP_NETWORK_INSTALLER:
>> +        osinfo_entity_set_param_boolean(OSINFO_ENTITY(media),
>> +                                        OSINFO_MEDIA_PROP_NETWORK_INSTALLER,
>> +                                        g_value_get_boolean(value));
>> +        break;
>> +
>
> I would expect a similar hunk in _get_property?
>
>>      default:
>>          /* We don't have any other property... */
>>          G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
>> @@ -616,7 +624,7 @@ osinfo_media_class_init(OsinfoMediaClass *klass)
>>                                   FALSE /* default value */,
>>                                   G_PARAM_READWRITE |
>>                                   G_PARAM_STATIC_STRINGS);
>> -    g_object_class_install_property(g_klass, PROP_LIVE, pspec);
>> +    g_object_class_install_property(g_klass, PROP_NETWORK_INSTALLER, pspec);
>>  }
>>
>>  static void
>>
>>
>> May I have your ack on this hunk or would you prefer if I send a v2
>> because of this change?
>
> Fine with me to squash this in.
>
> Christophe



-- 
Fabiano Fidêncio