Sure, I'm +1 on having this for public-facing headers.
That said, I don't think we should go overboard. In particular, it always
bugs me to see JavaDoc or Doxygen like:
/// Gets the start time of the operation.
/// @return the start time of the operation
public Time GetStartTime() {
return startTime;
}
In other words, I'm only in favor of docs that actually add real clarifying
value, rather than being an English restatement of the code. Given that,
I'm -1 on enforcing 'all parameters and return types should be doxygened'.
-Todd
On Tue, Jun 28, 2016 at 2:42 PM, Alexey Serbin <aser...@cloudera.com> wrote:
> Adar,
>
> Thank you for the analysis -- I'm glad you found it feasible to adopt the
> doxygen-like style for the header files at least :)
>
> There isn't 'standard' doxygen style -- it's just a set doxygen-specific
> keywords and formatting to allow doxygen to generate the docs (you can find
> more info on those at
> http://www.stack.nl/~dimitri/doxygen/manual/commands.html). From our
> side, that's more about choosing a set of conventions on _what_ we want to
> document and about _style_ of those comments (like using this or that
> commenting style, indenting multi-line comments, specifying a brief
> description for every public member and having detailed description
> optional, mandatory documentation for parameters and return values, etc.).
>
> As for the enforcement of the doxygen-like doc rules, I can see the
> following:
> * The doxygen tool itself generates warnings on non-documented member
> functions or non-documented parameters/return values, so that's easy to
> catch -- just make run of the doxygen tool as one of the build steps (like
> 'make doxygen-docs' or alike).
> * The style for the doxygen comments: I need to make some research on
> automating enforcement of that. Hopefully, we could have a custom config
> for vera++ or other C++ style checker to do that. I'll take a look at that
> and report on my findings. I think I could get some results here this or
> next week.
>
> BTW, if you want to see the auto-generated documentation, save the
> attached files at the same empty directory and run 'doxygen
> kudu_client.cfg' from that dir (on Mac, you could install doxygen via
> MacPorts or HomeBrew if not yet installed). Then, to browse the results,
> just open /tmp/kudu_docs/html/index.html in your favorite browser: you
> would be interested to click through the tabs and spend more time at the
> 'Data Structures' and its sub-tabs.
>
>
> Thanks,
>
> Alexey
>
> On Tue, Jun 28, 2016 at 1:24 PM, Adar Dembo <a...@cloudera.com> wrote:
>
>> Thanks for putting this together. I skimmed it and have a couple thoughts:
>> - Overall, this is less overhead than I thought it would be. I'm happy
>> about that.
>> - With the exception of annotating @param and @return correctly, it
>> seems like it won't be much work to adapt our current comments to this
>> style.
>> - I especially like the annotations for in and out parameters. That's
>> useful information that we don't communicate well (if at all) today,
>> so it's definitely an improvement.
>> - How much (if at all) does this deviate from "standard" doxygen style?
>> - I think it's really important to have a build-time checkstyle to
>> enforce this. Otherwise, either code reviews would become a drag or
>> our adherence to the syntax would deteriorate. Do you have any
>> thoughts on how we could do this?
>>
>>
>> On Tue, Jun 28, 2016 at 10:59 AM, Alexey Serbin <aser...@cloudera.com>
>> wrote:
>> > Hi,
>> >
>> > I re-formatted documentation in client.h file to be in doxygen-style.
>> > Please take a look at it to get an idea on the suggested style, etc.:
>> > https://gist.github.com/alexeyserbin/faad98b2ec9959acdf7256ff7d0bf139
>> > I also attached the file as is.
>> >
>> > Sure, this is just an initial draft -- let's communicate on different
>> > aspects and converge on the style conventions.
>> >
>> > Please let me know if you see there is something worth changing. Your
>> > feedback is highly appreciated!
>> >
>> >
>> > Thanks,
>> >
>> > Alexey
>> >
>> > On Thu, Jun 9, 2016 at 12:01 PM, Adar Dembo <a...@cloudera.com> wrote:
>> >>
>> >> We don't enforce much semantic consistency on our comments today. We
>> >> follow the Google style guide, and cpplint.py (tied to our 'make lint'
>> >> target) can enforce some stuff, but I don't think it does much, though
>> >> I'm not sure; I haven't really played around with it. So yeah, if we
>> >> enforce anything it's during code review, and that turns out to be
>> >> tedious and time consuming for everyone involved. Take a look at
>> >> cpplint.py; maybe it's got some checks that we're not running that'd
>> >> yield better consistency.
>> >>
>> >>
>> >>
>> >>
>> >> On Thu, Jun 9, 2016 at 11:51 AM, Alexey Serbin <aser...@cloudera.com>
>> >> wrote:
>> >> > Adar,
>> >> >
>> >> > I concur -- noisy and useless comments is an issue if not taken care
>> of.
>> >> >
>> >> > BTW, what about current comments in the code -- how is semantic
>> >> > consistency
>> >> > being enforced? I would think it's more about code-review time;
>> would
>> >> > not
>> >> > expect much automation there. As for the style, I also think
>> >> > style-checker
>> >> > would be great to have. Let's see what I can find out there. I
>> >> > remember
>> >> > there were some tools (vera++, etc.). From that regard I really
>> like Go
>> >> > lang approach on this :)
>> >> >
>> >> > Let me provide a snippet/sample of how that doxygen-styled comments
>> >> > would
>> >> > look like with current code and we can start from that.
>> >> >
>> >> >
>> >> > Thanks,
>> >> >
>> >> > Alexey
>> >> >
>> >> >
>> >> >
>> >> > On Thu, Jun 9, 2016 at 10:21 AM, Adar Dembo <a...@cloudera.com>
>> wrote:
>> >> >
>> >> >> I agree with Mike. In my experience Doxygen-style documentation can
>> be
>> >> >> very noisy in that it forces developers to write reams upon reams of
>> >> >> "obvious" documentation (i.e. adding a line for a @return that is
>> >> >> completely self-explanatory).
>> >> >>
>> >> >> I'm open to the idea of using it for public headers provided:
>> >> >> - It's easy to draw the line between those files and the rest of the
>> >> >> codebase.
>> >> >> - We have some script to automate checking the Doxygen style on
>> those
>> >> >> files.
>> >> >>
>> >> >> I'd also like us spend some time discussing which aspects of Doxygen
>> >> >> are worth implementing and which are excessive. Alexey, could you
>> >> >> drive this discussion? Not sure if it makes more sense to do it
>> over a
>> >> >> sample patch (to e.g. client.h) or in a vacuum, whatever you think
>> is
>> >> >> best.
>> >> >>
>> >> >> On Thu, Jun 9, 2016 at 9:57 AM, Alexey Serbin <aser...@cloudera.com
>> >
>> >> >> wrote:
>> >> >> > Hi All,
>> >> >> >
>> >> >> > Thank you for sharing your thoughts on this.
>> >> >> >
>> >> >> > From what I see the consensus is that for client API C++ headers
>> it
>> >> >> > makes
>> >> >> > sense to re-format in-code documentation to be in doxygen style.
>> So,
>> >> >> > I'm
>> >> >> > thinking about discussing style-related questions with Mike Percy
>> and
>> >> >> > others, preparing a patch and sending it for review.
>> >> >> >
>> >> >> > I think it's worth publishing the auto-generated results (HTML)
>> along
>> >> >> with
>> >> >> > client Java API docs. Misty, what help is needed there?
>> >> >> >
>> >> >> >
>> >> >> > Thanks,
>> >> >> >
>> >> >> > Alexey
>> >> >> >
>> >> >> > On Thu, Jun 9, 2016 at 9:37 AM, Misty Stanley-Jones <
>> >> >> > mstanleyjo...@cloudera.com> wrote:
>> >> >> >
>> >> >> >> I'm +1 too. Do we want to publish these as HTML like we do with
>> >> >> >> Javadoc
>> >> >> >> too? If so, who wants to volunteer to help me figure that out?
>> >> >> >>
>> >> >> >> On Thu, Jun 9, 2016 at 8:28 AM, Sameer Abhyankar <
>> >> >> sabhyan...@cloudera.com>
>> >> >> >> wrote:
>> >> >> >>
>> >> >> >> > +1 for the client facing APIs!
>> >> >> >> >
>> >> >> >> > Sameer Abhyankar
>> >> >> >> > Cloudera, Inc.
>> >> >> >> > (404) 431-7806
>> >> >> >> > sam...@cloudera.com
>> >> >> >> >
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > On Wed, Jun 8, 2016 at 10:25 PM, Mike Percy <mpe...@apache.org
>> >
>> >> >> wrote:
>> >> >> >> >
>> >> >> >> > > Hey Alexey,
>> >> >> >> > > Glad you brought it up. I'm inclined to agree that this
>> would be
>> >> >> >> helpful
>> >> >> >> > > for users of the C++ client APIs.
>> >> >> >> > >
>> >> >> >> > > But... I'm hesitant to do it for the rest of the code base.
>> It
>> >> >> >> > > can
>> >> >> be
>> >> >> >> > > pretty noisy, requires ongoing maintenance, and honestly I
>> don't
>> >> >> think
>> >> >> >> it
>> >> >> >> > > is helpful if you have decent dev tools set up (good editor
>> with
>> >> >> good
>> >> >> >> > > plugins, ack / ag, etc)
>> >> >> >> > >
>> >> >> >> > > If we adopt Doxygen, I think we should agree on a particular
>> >> >> >> > > style
>> >> >> and
>> >> >> >> > > adhere to it.
>> >> >> >> > >
>> >> >> >> > > Mike
>> >> >> >> > >
>> >> >> >> > >
>> >> >> >> > > On Wed, Jun 8, 2016 at 6:00 PM, Dan Burkert <
>> d...@cloudera.com>
>> >> >> wrote:
>> >> >> >> > >
>> >> >> >> > > > +1 from me for at least doing it in client.h and the few
>> other
>> >> >> public
>> >> >> >> > > > header files. These files have pretty much settled down,
>> so
>> >> >> >> > > > it
>> >> >> >> > shouldn't
>> >> >> >> > > > be too onerous to keep the doxygen comments up to date once
>> >> >> >> > > > they
>> >> >> are
>> >> >> >> > > > created.
>> >> >> >> > > >
>> >> >> >> > > > - Dan
>> >> >> >> > > >
>> >> >> >> > > > On Wed, Jun 8, 2016 at 4:36 PM, Alexey Serbin <
>> >> >> aser...@cloudera.com>
>> >> >> >> > > > wrote:
>> >> >> >> > > >
>> >> >> >> > > > > Hi All,
>> >> >> >> > > > >
>> >> >> >> > > > > Looking at current documentation for Kudu client API at
>> >> >> >> getkudu.org
>> >> >> >> > > > > you can see that Java API has auto-generated
>> documentation
>> >> >> >> > > > > but C++ API does not.
>> >> >> >> > > > >
>> >> >> >> > > > > Adding doxygen-style comments into C++ header files would
>> >> >> >> > > > > allow to auto-generate API documentation for C++ client
>> API
>> >> >> >> > > > > as
>> >> >> >> well.
>> >> >> >> > > > >
>> >> >> >> > > > > Assuming doxygen-formatted comments are not considered
>> >> >> >> > > > > as a violation of current C++ coding style,
>> >> >> >> > > > > would it make sense to introduce doxygen-style comments
>> >> >> >> > > > > for C++ client API headers?
>> >> >> >> > > > >
>> >> >> >> > > > > If it's worth it, in-line documentation in other parts of
>> >> >> >> > > > > the
>> >> >> C++
>> >> >> >> > code
>> >> >> >> > > > base
>> >> >> >> > > > > could be re-formatted into doxygen style as well.
>> >> >> >> > > > >
>> >> >> >> > > > > What do you think?
>> >> >> >> > > > >
>> >> >> >> > > > > Thank you!
>> >> >> >> > > > >
>> >> >> >> > > > >
>> >> >> >> > > > > Best regards,
>> >> >> >> > > > >
>> >> >> >> > > > > Alexey
>> >> >> >> > > > >
>> >> >> >> > > >
>> >> >> >> > >
>> >> >> >> >
>> >> >> >>
>> >> >>
>> >
>> >
>>
>
>
--
Todd Lipcon
Software Engineer, Cloudera
