You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kudu.apache.org by Alexey Serbin <as...@cloudera.com> on 2016/07/06 18:22:47 UTC
Re: Proposal: doxygen-style comments for C++ client API (and may be
other C++ components?)
Todd,
Thank you for the input!
In the context of example you provided, would the following be acceptable?
/// @return the start time of the operation
public Time GetStartTime() {
return startTime;
}
I.e., just documenting the return type and not duplicating that in the
description?
As Adar mentioned, it's worth having some constraints for automatic
validation, otherwise we are about to check both for presence and
semantic/clarifying value of those comments during code reviews. The
former can be automated if having requirement for documenting all return
types and parameters.
Out-of-the-box doxygen can automatically check for presence of comments on
public methods (except for constructors and destructors) and, as additional
option, that provided documentation have all parameters and return types
covered. From this perspective, having return type/value documented in the
snippet above is better than having description but no doc on return
type/value.
We can drop requirement to document all parameters and return type/value,
but then we need to check for presence of valuable ones during codereviews,
no automation here. Is it ok?
Thanks,
Alexey
On Thu, Jun 30, 2016 at 4:05 PM, Todd Lipcon <to...@cloudera.com> wrote:
> 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 <as...@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 <ad...@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 <as...@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 <ad...@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 <aserbin@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 <ad...@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 <
> aserbin@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 <
> >> >> >> > mstanleyjones@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 <
> >> >> >> sabhyankar@cloudera.com>
> >> >> >> >> wrote:
> >> >> >> >>
> >> >> >> >> > +1 for the client facing APIs!
> >> >> >> >> >
> >> >> >> >> > Sameer Abhyankar
> >> >> >> >> > Cloudera, Inc.
> >> >> >> >> > (404) 431-7806
> >> >> >> >> > sameer@cloudera.com
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> > On Wed, Jun 8, 2016 at 10:25 PM, Mike Percy <
> mpercy@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 <
> >> dan@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 <
> >> >> >> aserbin@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
>
Re: Proposal: doxygen-style comments for C++ client API (and may be
other C++ components?)
Posted by Alexey Serbin <as...@cloudera.com>.
Mike,
Thank you for the input!
I think there are some outstanding issues with that (commend style
checker), but I've just sent the change for review -- please take a look:
https://gerrit.cloudera.org/#/c/3619/
Having this in place would allow to generate web-oriented documentation
which we could be accessible after publishing it at the Kudu site.
Meanwhile, I can deal with the outstanding minor issues in the background.
Best regards,
Alexey
On Mon, Jul 11, 2016 at 3:31 PM, Mike Percy <mp...@apache.org> wrote:
> On Wed, Jul 6, 2016 at 12:56 PM, Alexey Serbin <as...@cloudera.com>
> wrote:
>
> > On Wed, Jul 6, 2016 at 11:36 AM, Todd Lipcon <to...@cloudera.com> wrote:
> >
> > > On Wed, Jul 6, 2016 at 11:22 AM, Alexey Serbin <as...@cloudera.com>
> > > wrote:
> > >
> > > > Todd,
> > > >
> > > > Thank you for the input!
> > > >
> > > > In the context of example you provided, would the following be
> > > acceptable?
> > > >
> > > > /// @return the start time of the operation
> > > > public Time GetStartTime() {
> > > > return startTime;
> > > > }
> > > >
> > > > I.e., just documenting the return type and not duplicating that in
> the
> > > > description?
> > > >
> > >
> > > Personally I find that silly, too, since it's an inline function and
> it's
> > > typically obvious what a getter does. But, if people think it's useful,
> > I'm
> > > not going to fight about it :)
> >
>
> I agree with Todd that this example is pretty useless. :)
>
> ... snip another code example with more parameters ...
>
> > In fact, IMO it actually has a negative impact on the readability of the
> > > code, because I can fit less of the API on my screen at a time and I
> have
> > > to "skip over" the filler comments.
> > >
> > > This example comes from an internal-facing class though, and I can see
> > the
> > > argument for using the more verbose doxygen style for the public
> > *exported*
> > > API. To clarify, when you say "public APIs" do you mean "public" from a
> > C++
> > > perspective? Or "public" from a "we will maintain this as an API for
> > > consumers of the Kudu client" perspective?
> >
> > In this context that mean 'public from C++ perspective in files which we
> > want to be processed by doxygen to auto-generate docs for customer-facing
> > APIs' :)
> >
> > I.e., that's about public C++ members in files which we decide to be
> > documented this way. At current state, that's about public C++ members
> of
> > classes/namespaces in src/kudu/client.h file only.
> >
>
> This sounds good to me as a guideline, but I still think we should use our
> own judgment.
>
> > > We can drop requirement to document all parameters and return
> > type/value,
> > > > but then we need to check for presence of valuable ones during
> > > codereviews,
> > > > no automation here. Is it ok?
> > > >
> > > >
> > > I'm personally OK without automation, since I think comments are meant
> > for
> > > human consumption, and humans are probably the best judge of what a
> good
> > > description should be. In other words, even if doxygen verifies that a
> > > function is "well commented", the reviewer still has to look at the
> > comment
> > > and make sure it actually makes sense, fully describes anything tricky,
> > > etc.
> >
> > Exactly -- making sure the documentation have some value is on us and
> that
> > verification is to be done during code review. However, it's possible to
> > introduce automatic verification of the _presence_ of corresponding
> entries
> > to catch issues with stale/absent items after re-factoring and adding a
> new
> > code. And that can be done automatically by doxygen.
>
>
> > So, do you think it's worth adding that automated checks by doxygen just
> > for _presence_ of documentation for parameters and return types/values?
> > That implies every public method in client.h file must have
> > parameters/return documented. :)
> >
>
> No, I don't think we should enable a rule that all public methods must have
> Doxygen, for exactly the reasons above. Since we're discussing only having
> doxygen on a single header file, I don't think manual review would be
> onerous.
>
> If we're in agreement, and to me it sounds like we're converging on that,
> let's do a first pass on this on Gerrit. That will allow us to take a look
> at the updated code together as well as the rendered doxygen HTML. I think
> that will inform our opinions and is likely to lead to consensus on this
> topic.
>
> Mike
>
Re: Proposal: doxygen-style comments for C++ client API (and may be
other C++ components?)
Posted by Mike Percy <mp...@apache.org>.
On Wed, Jul 6, 2016 at 12:56 PM, Alexey Serbin <as...@cloudera.com> wrote:
> On Wed, Jul 6, 2016 at 11:36 AM, Todd Lipcon <to...@cloudera.com> wrote:
>
> > On Wed, Jul 6, 2016 at 11:22 AM, Alexey Serbin <as...@cloudera.com>
> > wrote:
> >
> > > Todd,
> > >
> > > Thank you for the input!
> > >
> > > In the context of example you provided, would the following be
> > acceptable?
> > >
> > > /// @return the start time of the operation
> > > public Time GetStartTime() {
> > > return startTime;
> > > }
> > >
> > > I.e., just documenting the return type and not duplicating that in the
> > > description?
> > >
> >
> > Personally I find that silly, too, since it's an inline function and it's
> > typically obvious what a getter does. But, if people think it's useful,
> I'm
> > not going to fight about it :)
>
I agree with Todd that this example is pretty useless. :)
... snip another code example with more parameters ...
> In fact, IMO it actually has a negative impact on the readability of the
> > code, because I can fit less of the API on my screen at a time and I have
> > to "skip over" the filler comments.
> >
> > This example comes from an internal-facing class though, and I can see
> the
> > argument for using the more verbose doxygen style for the public
> *exported*
> > API. To clarify, when you say "public APIs" do you mean "public" from a
> C++
> > perspective? Or "public" from a "we will maintain this as an API for
> > consumers of the Kudu client" perspective?
>
> In this context that mean 'public from C++ perspective in files which we
> want to be processed by doxygen to auto-generate docs for customer-facing
> APIs' :)
>
> I.e., that's about public C++ members in files which we decide to be
> documented this way. At current state, that's about public C++ members of
> classes/namespaces in src/kudu/client.h file only.
>
This sounds good to me as a guideline, but I still think we should use our
own judgment.
> > We can drop requirement to document all parameters and return
> type/value,
> > > but then we need to check for presence of valuable ones during
> > codereviews,
> > > no automation here. Is it ok?
> > >
> > >
> > I'm personally OK without automation, since I think comments are meant
> for
> > human consumption, and humans are probably the best judge of what a good
> > description should be. In other words, even if doxygen verifies that a
> > function is "well commented", the reviewer still has to look at the
> comment
> > and make sure it actually makes sense, fully describes anything tricky,
> > etc.
>
> Exactly -- making sure the documentation have some value is on us and that
> verification is to be done during code review. However, it's possible to
> introduce automatic verification of the _presence_ of corresponding entries
> to catch issues with stale/absent items after re-factoring and adding a new
> code. And that can be done automatically by doxygen.
> So, do you think it's worth adding that automated checks by doxygen just
> for _presence_ of documentation for parameters and return types/values?
> That implies every public method in client.h file must have
> parameters/return documented. :)
>
No, I don't think we should enable a rule that all public methods must have
Doxygen, for exactly the reasons above. Since we're discussing only having
doxygen on a single header file, I don't think manual review would be
onerous.
If we're in agreement, and to me it sounds like we're converging on that,
let's do a first pass on this on Gerrit. That will allow us to take a look
at the updated code together as well as the rendered doxygen HTML. I think
that will inform our opinions and is likely to lead to consensus on this
topic.
Mike
Re: Proposal: doxygen-style comments for C++ client API (and may be
other C++ components?)
Posted by Alexey Serbin <as...@cloudera.com>.
On Wed, Jul 6, 2016 at 11:36 AM, Todd Lipcon <to...@cloudera.com> wrote:
> On Wed, Jul 6, 2016 at 11:22 AM, Alexey Serbin <as...@cloudera.com>
> wrote:
>
> > Todd,
> >
> > Thank you for the input!
> >
> > In the context of example you provided, would the following be
> acceptable?
> >
> > /// @return the start time of the operation
> > public Time GetStartTime() {
> > return startTime;
> > }
> >
> > I.e., just documenting the return type and not duplicating that in the
> > description?
> >
>
> Personally I find that silly, too, since it's an inline function and it's
> typically obvious what a getter does. But, if people think it's useful, I'm
> not going to fight about it :)
>
>
> >
> > As Adar mentioned, it's worth having some constraints for automatic
> > validation, otherwise we are about to check both for presence and
> > semantic/clarifying value of those comments during code reviews. The
> > former can be automated if having requirement for documenting all return
> > types and parameters.
> >
> > Out-of-the-box doxygen can automatically check for presence of comments
> on
> > public methods (except for constructors and destructors) and, as
> additional
> > option, that provided documentation have all parameters and return types
> > covered. From this perspective, having return type/value documented in
> the
> > snippet above is better than having description but no doc on return
> > type/value.
> >
>
> Sure, but to take another example (from tablet.h):
>
> // Create a new row iterator for some historical snapshot.
> Status NewRowIterator(const Schema &projection,
> const MvccSnapshot &snap,
> const OrderMode order,
> gscoped_ptr<RowwiseIterator> *iter) const;
>
> To me, it doesn't add any value to rewrite this comment as:
> /// Create a new row iterator for some historical snapshot.
> /// @param projection the projection
> /// @param snap the snapshot
> /// @param order the order mode (see OrderMode enum)
> /// @param [out] iter the returned iterator
> Status NewRowIterator(const Schema &projection,
> const MvccSnapshot &snap,
> const OrderMode order,
> gscoped_ptr<RowwiseIterator> *iter) const;
>
> In fact, IMO it actually has a negative impact on the readability of the
> code, because I can fit less of the API on my screen at a time and I have
> to "skip over" the filler comments.
>
> This example comes from an internal-facing class though, and I can see the
> argument for using the more verbose doxygen style for the public *exported*
> API. To clarify, when you say "public APIs" do you mean "public" from a C++
> perspective? Or "public" from a "we will maintain this as an API for
> consumers of the Kudu client" perspective?
In this context that mean 'public from C++ perspective in files which we
want to be processed by doxygen to auto-generate docs for customer-facing
APIs' :)
I.e., that's about public C++ members in files which we decide to be
documented this way. At current state, that's about public C++ members of
classes/namespaces in src/kudu/client.h file only.
>
>
> >
> > We can drop requirement to document all parameters and return type/value,
> > but then we need to check for presence of valuable ones during
> codereviews,
> > no automation here. Is it ok?
> >
> >
> I'm personally OK without automation, since I think comments are meant for
> human consumption, and humans are probably the best judge of what a good
> description should be. In other words, even if doxygen verifies that a
> function is "well commented", the reviewer still has to look at the comment
> and make sure it actually makes sense, fully describes anything tricky,
> etc.
>
Exactly -- making sure the documentation have some value is on us and that
verification is to be done during code review. However, it's possible to
introduce automatic verification of the _presence_ of corresponding entries
to catch issues with stale/absent items after re-factoring and adding a new
code. And that can be done automatically by doxygen.
So, do you think it's worth adding that automated checks by doxygen just
for _presence_ of documentation for parameters and return types/values?
That implies every public method in client.h file must have
parameters/return documented. :)
As an option, we can require presence of some comment for a method/member,
and no checks for parameters/returns. That check can be performed
automatically by doxygen.
BTW, styling of those comments (like spacing, etc.) is a separate thing,
and that is not handled by doxygen. For that it might required to run some
style-checker tool (vera++, cppcheck, cpplint, etc.) but none of those I
checked has something ready to use in their stock distribution. I would
suggest to go without those styles first, but add them later (e.g., we
could implement those for vera++ as custom rules in TCL).
>
>
> >
> > Thanks,
> >
> > Alexey
> >
> > On Thu, Jun 30, 2016 at 4:05 PM, Todd Lipcon <to...@cloudera.com> wrote:
> >
> > > 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 <as...@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 <ad...@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 <
> aserbin@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 <ad...@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 <
> > aserbin@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 <adar@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 <
> > > aserbin@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 <
> > > >> >> >> > mstanleyjones@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 <
> > > >> >> >> sabhyankar@cloudera.com>
> > > >> >> >> >> wrote:
> > > >> >> >> >>
> > > >> >> >> >> > +1 for the client facing APIs!
> > > >> >> >> >> >
> > > >> >> >> >> > Sameer Abhyankar
> > > >> >> >> >> > Cloudera, Inc.
> > > >> >> >> >> > (404) 431-7806
> > > >> >> >> >> > sameer@cloudera.com
> > > >> >> >> >> >
> > > >> >> >> >> >
> > > >> >> >> >> >
> > > >> >> >> >> > On Wed, Jun 8, 2016 at 10:25 PM, Mike Percy <
> > > mpercy@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 <
> > > >> dan@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 <
> > > >> >> >> aserbin@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
> > >
> >
>
>
>
> --
> Todd Lipcon
> Software Engineer, Cloudera
>
Re: Proposal: doxygen-style comments for C++ client API (and may be
other C++ components?)
Posted by Todd Lipcon <to...@cloudera.com>.
On Wed, Jul 6, 2016 at 11:22 AM, Alexey Serbin <as...@cloudera.com> wrote:
> Todd,
>
> Thank you for the input!
>
> In the context of example you provided, would the following be acceptable?
>
> /// @return the start time of the operation
> public Time GetStartTime() {
> return startTime;
> }
>
> I.e., just documenting the return type and not duplicating that in the
> description?
>
Personally I find that silly, too, since it's an inline function and it's
typically obvious what a getter does. But, if people think it's useful, I'm
not going to fight about it :)
>
> As Adar mentioned, it's worth having some constraints for automatic
> validation, otherwise we are about to check both for presence and
> semantic/clarifying value of those comments during code reviews. The
> former can be automated if having requirement for documenting all return
> types and parameters.
>
> Out-of-the-box doxygen can automatically check for presence of comments on
> public methods (except for constructors and destructors) and, as additional
> option, that provided documentation have all parameters and return types
> covered. From this perspective, having return type/value documented in the
> snippet above is better than having description but no doc on return
> type/value.
>
Sure, but to take another example (from tablet.h):
// Create a new row iterator for some historical snapshot.
Status NewRowIterator(const Schema &projection,
const MvccSnapshot &snap,
const OrderMode order,
gscoped_ptr<RowwiseIterator> *iter) const;
To me, it doesn't add any value to rewrite this comment as:
/// Create a new row iterator for some historical snapshot.
/// @param projection the projection
/// @param snap the snapshot
/// @param order the order mode (see OrderMode enum)
/// @param [out] iter the returned iterator
Status NewRowIterator(const Schema &projection,
const MvccSnapshot &snap,
const OrderMode order,
gscoped_ptr<RowwiseIterator> *iter) const;
In fact, IMO it actually has a negative impact on the readability of the
code, because I can fit less of the API on my screen at a time and I have
to "skip over" the filler comments.
This example comes from an internal-facing class though, and I can see the
argument for using the more verbose doxygen style for the public *exported*
API. To clarify, when you say "public APIs" do you mean "public" from a C++
perspective? Or "public" from a "we will maintain this as an API for
consumers of the Kudu client" perspective?
>
> We can drop requirement to document all parameters and return type/value,
> but then we need to check for presence of valuable ones during codereviews,
> no automation here. Is it ok?
>
>
I'm personally OK without automation, since I think comments are meant for
human consumption, and humans are probably the best judge of what a good
description should be. In other words, even if doxygen verifies that a
function is "well commented", the reviewer still has to look at the comment
and make sure it actually makes sense, fully describes anything tricky, etc.
>
> Thanks,
>
> Alexey
>
> On Thu, Jun 30, 2016 at 4:05 PM, Todd Lipcon <to...@cloudera.com> wrote:
>
> > 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 <as...@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 <ad...@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 <aserbin@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 <ad...@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 <
> aserbin@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 <ad...@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 <
> > aserbin@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 <
> > >> >> >> > mstanleyjones@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 <
> > >> >> >> sabhyankar@cloudera.com>
> > >> >> >> >> wrote:
> > >> >> >> >>
> > >> >> >> >> > +1 for the client facing APIs!
> > >> >> >> >> >
> > >> >> >> >> > Sameer Abhyankar
> > >> >> >> >> > Cloudera, Inc.
> > >> >> >> >> > (404) 431-7806
> > >> >> >> >> > sameer@cloudera.com
> > >> >> >> >> >
> > >> >> >> >> >
> > >> >> >> >> >
> > >> >> >> >> > On Wed, Jun 8, 2016 at 10:25 PM, Mike Percy <
> > mpercy@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 <
> > >> dan@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 <
> > >> >> >> aserbin@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
> >
>
--
Todd Lipcon
Software Engineer, Cloudera