On Mon, Mar 7, 2011 at 7:33 PM, Angus Salkeld <[email protected]> wrote: > Hi Russell > > Some gneral comments:
Thank you for the feedback! > 1) > I don't think you need all those "\brief" keywords. I think they look > quite ugly in the code. Below works fine for me and is cleaner. > > /** > * this is always a brief > * > * leave a line and this is the vebose description. > */ Oh, neat. For some reason I thought the brief keyword was required. I agree that it looks nicer without it if not required. > 2) > I think there little point publishing the exec/*.h files. I know you can > write your own corosync based on these but I think > these would probably succeed in confusing the reader more than it would help. > > Steven you think any different? One question to ask is who is the documentation written for? If it's only for developers of applications that use corosync APIs, that's one thing. Is it also for developers of corosync itself? That may change what content you want to have in or out. Assuming the documentation is for both developers against public APIs and for corosync itself, I would propose leaving all files in. It would be nice to have some additional documentation built into the main page that said "here are the APIs you should care about" for application developers and leave the rest in the background. > 3) > we are currently using /** @<keyword> */ not /** \<keyword> */ No problem. I just used \keyword out of habit. I'll change it. >> +/** >> + * \brief Get a file descriptor on which to poll. >> + * >> + * confdb_handle_t is NOT a file descriptor and may not be used directly. > > Strange comment? fd is a real file descriptor. The handle is an input > not an output. > I know it was already there, but we should remove this. I can remove it in the next patch. I parsed it as trying to convey why the function is necessary. You can't use the handle directly, you need to get an fd using this function? *shrugs* That seems pretty obvious to me, though. Thanks, -- Russell Bryant _______________________________________________ Openais mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/openais
