Re: [Qemu-devel] [PATCH 2/3] asn1 ber visitors

[mdroth]

On Wed, Feb 27, 2013 at 02:03:45PM -0600, Joel Schopp wrote:
> Thanks for the feedback.
> 
> On 02/27/2013 02:28 AM, Andreas Färber wrote:
> >Am 27.02.2013 00:03, schrieb address@hidden:
> >>These patches implement asn1 ber visitors for encoding and decoding data.
> >
> >Would be good to not be lazy and spell them correctly in at least one of
> >the two lines of the commit where they're being introduced.
> 
> I will change "asn1 ber" to "ASN.1 BER"
> 
> >I would suggest to place your headers into existing include/qapi/ for
> >instance and the remaining source files into qapi/ or a subdirectory
> >thereof rather than a new top-level directory.
> >
> >I also note that you're not changing MAINTAINERS, so the new files don't
> >have a maintainer assigned that gets CC'ed on changes.
> 
> I'll update MAINTAINERS and move the headers into include.
> 
> As for the location of the source files I think where the patch puts
> them now makes most sense, I don't think qapi makes sense.  But the
> location isn't important to me so I'll put them wherever.  Anybody
> else have an opinion on the best location?

qapi/ along with all the other visitor implementations is probably the
right place for now. If in the future we end up with dozens of
implementations pulling into ridiculous amounts of dependencies we might
want to reconsider things, but for now it's safest to assume that any
serialization format for can be selected at runtime for a Visitor user,
so they should all be made available via qapi-obj-y.

It's also possible this might fix whatever build issue you're running into
for free :)