Kevin Wolf <kw...@redhat.com> writes: > Am 18.11.2015 um 17:26 hat Eric Blake geschrieben: >> On 11/18/2015 05:08 AM, Kevin Wolf wrote: >> > Am 18.11.2015 um 11:30 hat Markus Armbruster geschrieben: >> >> Eric Blake <ebl...@redhat.com> writes: >> >> >> >>> No need to keep two separate enums, where editing one is likely >> >>> to forget the other. Now that we can specify a qapi enum prefix, >> >>> we don't even have to change the bulk of the uses. >> >>> >> >> >>> ## >> >>> -{ 'enum': 'BlkdebugEvent', >> >>> +{ 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG', >> >>> 'data': [ 'l1_update', 'l1_grow.alloc_table', 'l1_grow.write_table', >> >>> 'l1_grow.activate_table', 'l2_load', 'l2_update', >> >>> 'l2_update_compressed', 'l2_alloc.cow_read', >> >>> 'l2_alloc.write', >> >> >> >> I'm no friend of the 'prefix' feature. You could avoid it here by >> >> renaming BlkdebugEvent to Blkdbg. No additional churn, because you >> >> already replace hand-written BlkDebugEvent by generated BlkdebugEvent. >> >> >> >> Alternatively, you could reduce churn by renaming it to BlkDebugEvent. >> >> >> >> Matter of taste. Perhaps Kevin has a preference. >> > >> > Wouldn't that mean that we end up with a C type called Blkdbg? In my >> > opinion that's a bit unspecific, so if you ask me, I would paint my >> > bikeshed in a different colour. >> >> Most of the existing qapi names are Blkdebug, it was only the internal >> enum being replaced that used BlkDebug. Also, qapi would munge BlkDebug >> into BLK_DEBUG, so I think we want to stick with the lowercase 'd' so >> that the munging (without 'prefix') would stick to BLKDEBUG. > > Oh, I don't really have an opinion whether BlockDebug, BlkDebug or > Blkdebug is the best spelling. I think Blkdebug makes sense. > > But I do have a strong opinion on whether the enum should be called > BlkdebugEvent or just Blkdbg (without any mention of "Event"). The > latter doesn't describe any more what the type is about.
Okay, we'll take the patch as is then. >> I'm also fine if you want me to do a followup patch that renames all >> enums from BLKDBG_L1_UPDATE to BLKDEBUG_EVENT_L1_UPDATE, at which point >> we could drop the 'prefix' (I don't know how many lines it would make >> long enough to need different wrapping, but modulo wrapping it would all >> be a mechanically scripted change). > > I don't mind the 'prefix' feature. I think it's nice to have short, but > still unique enough constant names, but if Markus is bothered enough to > send a patch, I can live with BLKDEBUG_EVENT_*. Fair enough :) For now, let's move the queue forward.