On Fri, Jun 10, 2016 at 09:57:13AM -0400, Cole Robinson wrote:
> On 06/10/2016 09:50 AM, Ján Tomko wrote:
> > On Fri, Jun 10, 2016 at 08:30:33AM -0400, Cole Robinson wrote:
> >> On 06/10/2016 07:05 AM, Ján Tomko wrote:
> >>> On Thu, Jun 09, 2016 at 05:43:05PM -0400, Cole Robinson wrote:
> >>>> On 06/09/2016 02:25 PM, Jovanka Gulicoska wrote:
> >>>>> + virReportInvalidArg(eventID,
> >>>>> + _("eventID in %s must be less than %d"),
> >>>>> + __FUNCTION__, VIR_STORAGE_POOL_EVENT_ID_LAST);
> >>>>> + goto error;
> >>>>> + }
> >>>>> +
> >>>>> + if (conn->storageDriver &&
> >>>>> conn->storageDriver->connectStoragePoolEventRegisterAny) {
> >>>>
> >>>> Long line, split it after the &&
> >>>>
> >>>>> + int ret;
> >>>>> + ret =
> >>>>> conn->storageDriver->connectStoragePoolEventRegisterAny(conn, pool,
> >>>>> +
> >>>>> eventID,
> >>>>> +
> >>>>> cb, opaque,
> >>>>> +
> >>>>> freecb);
> >>>>
> >>>> If you put pool and opaque on their own lines this breaks some over 80 column
> >>>> lines, but I don't know if that's better or worse
> >>>>
> >>>
> >>> The 'Any' suffix can also be dropped.
> >>> There is no legacy virConnectStoragePoolEventRegister API so we don't
> >>> need to add suffixes like we had to for virConnectDomainEventRegister.
> >>>
> >>
> >> You mean the internal driver name 'connectStoragePoolEventRegisterAny', not
> >> the public API?. If so that sounds fine to me, but I'm not sure how much
> >> precedent there is for having driver function names differ from the public API
> >> names
> >
> > I meant both the public API and the internal driver name.
> >
>
> I disagree, that will make the public API inconsistent across object types. We
> would end up with basically
>
> DomainEventRegister(conn, cb)
> DomainEventRegisterAny(conn, obj, eventID, cb)
> NetworkEventRegisterAny(conn, obj, eventID, cb)
> StoragePoolEventRegister(conn, obj, eventID, cb)
>
> So not only would the preferred API not have consistent naming, the similarly
> named APIs would have different signatures. I don't think that's worth it to
> to get slightly nicer named APIs for storage events
Yep, I'm with Cole on this - having consistent naming for the different
types of objects is better IMHO, so we should *keep* the "Any" suffix on
these new methods
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list