On Wed, Apr 17, 2013 at 10:16:08AM -0600, Eric Blake wrote: > On 04/17/2013 09:43 AM, Daniel P. Berrange wrote: > > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx> > > > > Currently the RPC protocol files can contain annotations after > > the protocol enum eg > > > > REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, /* autogen autogen priority:high */ > > > > This is not very extensible as the number of annotations grows. > > Hmm - wonder if that means you plan on adding annotations soon :) You're not wrong ! I'll be adding annotations to describe the access control rules for each API call, so that we can auto-generate methods for doing access control checks in each driver. > > Change it to use > > > > /** > > * @generate: both > > * @priority: high > > */ > > REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, > > At which point grouping by 10 no longer matters. I guess, I was in two minds as to whether to remove that grouping or not. > > - VIR_LOCK_SPACE_PROTOCOL_PROC_REGISTER = 1, /* skipgen skipgen */ > > - VIR_LOCK_SPACE_PROTOCOL_PROC_RESTRICT = 2, /* skipgen skipgen */ > > - VIR_LOCK_SPACE_PROTOCOL_PROC_NEW = 3, /* skipgen skipgen */ > > - VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_RESOURCE = 4, /* skipgen skipgen */ > > - VIR_LOCK_SPACE_PROTOCOL_PROC_DELETE_RESOURCE = 5, /* skipgen skipgen */ > > + /** > > + * @generate: none > > + */ > > + VIR_LOCK_SPACE_PROTOCOL_PROC_REGISTER = 1, > > + /** > > I think it would be nicer to put a blank line before the /** of each > listing, so that it is even more visually obvious that a comment applies > to the enum below it. Yeah, a good idea. > > > + * @generate: none > > + */ > > + VIR_LOCK_SPACE_PROTOCOL_PROC_RESTRICT = 2, > > + /** > > + * @generate: none > > + */ > > + VIR_LOCK_SPACE_PROTOCOL_PROC_NEW = 3, > > + /** > > + * @generate: none > > + */ > > + VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_RESOURCE = 4, > > + /** > > + * @generate: none > > + */ > > + VIR_LOCK_SPACE_PROTOCOL_PROC_DELETE_RESOURCE = 5, > > > > - VIR_LOCK_SPACE_PROTOCOL_PROC_ACQUIRE_RESOURCE = 6, /* skipgen skipgen */ > > - VIR_LOCK_SPACE_PROTOCOL_PROC_RELEASE_RESOURCE = 7, /* skipgen skipgen */ > > + /** > > Hmm, your conversion preserved existing blank lines, but if you take my > advice of adding a blank before every /**, I don't see any point in > having double blanks before multiples of 5 or 10. Ok. 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