On Sat, 27 Jun 2020 at 12:53, Markus Armbruster <arm...@redhat.com> wrote: > Peter Maydell <peter.mayd...@linaro.org> writes: > > Hi. I've just noticed that this commit added new global-scope > > functions to header files without documentation comments, eg > > > > bool qdev_realize_and_unref(DeviceState *dev, BusState *bus, Error **errp); > > They actually have doc comments: next to their definition, just like the > functions they replace.
> > Please could you provide some follow-up patches that document them? > > I don't think we have any hope of getting people to follow whatever > > the correct new way to create/configure/realize devices is if we > > don't document it :-( [Concrete example: I now have no idea > > how this is supposed to work after this patchset.] > > Please check out my function comments, and if they leave you confused, > let's talk about improvements. I will have a look at them, but we should move them (wholesale with other documentation comments for qdev) to the header files. (That we are having this discussion at all demonstrates why -- people don't look in the C files for API documentation of functions.) The headers are where the API that faces the rest of QEMU should be documented; comments in the C files are for people who care about the internals of the implementation. "New prototype in a header file should have a doc comment" is a standard part of my code review I apply to any code which I see going past. We absolutely have not been good about documenting our facing-the-rest-of-QEMU functions in the past but this is an area where I think we should be raising the bar. > I'm content to use comment placement / formatting I dislike to make my > code blend in, but I'm not willing to do conversion work from a style I > like to style I dislike. That's a job for someone who won't feel icky > afterwards :) Fair enough. I'm happy to write some patches to consistently put all the qdev doc info into the headers. thanks -- PMM
