You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Attila Bukor (Code Review)" <ge...@cloudera.org> on 2020/08/26 12:02:58 UTC

[kudu-CR] Fix C++ client API docs 

Attila Bukor has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16372


Change subject: Fix C++ client API docs
......................................................................

Fix C++ client API docs

This patch fixes C++ API doxygen comments so the build succeeds.

Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
---
M src/kudu/client/client.h
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema.h
M src/kudu/client/value.h
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/util/monotime.h
M src/kudu/util/status.h
9 files changed, 170 insertions(+), 171 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/16372/1
-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................

Fix C++ client API docs

This patch fixes C++ API doxygen comments so the build succeeds. It's
unclear when this broke exactly.

Running docs/support/scripts/make_site.sh with Doxygen 1.8.11 failed
with the below warning:

util/monotime.h:371: error: argument 'lhs' of command @param is not found in the argument list of kudu::MonoDelta::operator+(const MonoDelta &, const MonoDelta &) (warning treated as error, aborting now)

When Kudu 1.12.0 was released, the docs were generated with Doxygen
1.8.11, although it's possible that treating warnings as errors was
disabled for the release to work around this issue.

After upgrading to Doxygen 1.8.15 this warning disappeared, but a
different one surfaced:

common/partial_row.h:105: error: Member SetBool(const Slice &col_name, bool val) WARN_UNUSED_RESULT (function) of class KuduPartialRow is not documented. (warning treated as error, aborting now)

I disabled WARN_AS_ERROR temporarily only to discover several other
warnings. Some of them were due to Doxygen bugs solved by another update
to 1.8.19 in commit de31237, the rest manually by this commit.

Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Reviewed-on: http://gerrit.cloudera.org:8080/16372
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
---
M src/kudu/client/client.h
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema.h
M src/kudu/client/value.h
M src/kudu/common/partial_row.h
M src/kudu/util/monotime.h
M src/kudu/util/status.h
8 files changed, 264 insertions(+), 156 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Grant Henke: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 8
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] Fix C++ client API docs

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16372/1/src/kudu/client/scan_batch.h
File src/kudu/client/scan_batch.h:

http://gerrit.cloudera.org:8080/#/c/16372/1/src/kudu/client/scan_batch.h@a211
PS1, Line 211: 
We lose a bit of "documentation" here given I think this was the only place we described what the unit of the value was. Maybe this needs it's own documentation block?


http://gerrit.cloudera.org:8080/#/c/16372/1/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:

http://gerrit.cloudera.org:8080/#/c/16372/1/src/kudu/common/partial_row.h@a111
PS1, Line 111: 
             : 
             : 
             : 
Same as my comment in scan_batch.


http://gerrit.cloudera.org:8080/#/c/16372/1/src/kudu/common/partial_row.h@a452
PS1, Line 452: 
             : 
             : 
Same as my comment in scan_batch.



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 Aug 2020 13:51:39 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16372/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16372/2//COMMIT_MSG@9
PS2, Line 9: the build succeeds
> Thank you for fixing these doxygen issues.
Yes, I didn't see anything off.



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 Aug 2020 16:43:52 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 7: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 7
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 27 Aug 2020 13:50:45 +0000
Gerrit-HasComments: No

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16372

to look at the new patch set (#2).

Change subject: Fix C++ client API docs
......................................................................

Fix C++ client API docs

This patch fixes C++ API doxygen comments so the build succeeds.

Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
---
M src/kudu/client/client.h
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema.h
M src/kudu/client/value.h
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/util/monotime.h
M src/kudu/util/status.h
9 files changed, 263 insertions(+), 156 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/16372/2
-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] Fix C++ client API docs

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16372/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16372/3//COMMIT_MSG@29
PS3, Line 29: I5eddf689da90123ca52075d09de07981d11f8ffe
nit: I guess it's better to use githash while referencing changelists



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 Aug 2020 18:05:12 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16372/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16372/2//COMMIT_MSG@9
PS2, Line 9: the build succeeds
> Could you mention the version of doxygen or such in the commit message beca
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 Aug 2020 22:24:57 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16372

to look at the new patch set (#3).

Change subject: Fix C++ client API docs
......................................................................

Fix C++ client API docs

This patch fixes C++ API doxygen comments so the build succeeds. It's
unclear when this broke exactly.

Running docs/support/scripts/make_site.sh with Doxygen 1.8.11 failed
with the below warning:

util/monotime.h:371: error: argument 'lhs' of command @param is not found in the argument list of kudu::MonoDelta::operator+(const MonoDelta &, const MonoDelta &) (warning treated as error, aborting now)

When Kudu 1.12.0 was released, the docs were generated with Doxygen
1.8.11, although it's possible that treating warnings as errors was
disabled for the release to work around this issue.

After upgrading to Doxygen 1.8.15 this warning disappeared, but a
different one surfaced:

common/partial_row.h:105: error: Member SetBool(const Slice &col_name, bool val) WARN_UNUSED_RESULT (function) of class KuduPartialRow is not documented. (warning treated as error, aborting now)

I disabled WARN_AS_ERROR temporarily only to discover several other
warnings. Some of them were due to Doxygen bugs solved by another update
to 1.8.19 in an earlier commit (change ID:
I5eddf689da90123ca52075d09de07981d11f8ffe), the rest manually by this
commit.

Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
---
M src/kudu/client/client.h
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema.h
M src/kudu/client/value.h
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/util/monotime.h
M src/kudu/util/status.h
9 files changed, 263 insertions(+), 156 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/16372/3
-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] Fix C++ client API docs

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 6: Code-Review+2

(3 comments)

LGTM, just a few more nits that I didn't noticed during the first review.

http://gerrit.cloudera.org:8080/#/c/16372/6/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/16372/6/src/kudu/client/client.h@1123
PS6, Line 1123: @note This method is experimental and may change or disappear in future.
nit: maybe it makes sense to make this and warning at line 1173 uniform, i.e. have the same @note or @warning in both places.


http://gerrit.cloudera.org:8080/#/c/16372/6/src/kudu/client/client.h@2438
PS6, Line 2438:   /// @name Advanced/Unstable API
              :   //
              :   ///@{
              :   /// Modifier flags for the row format returned from the server.
              :   ///
nit: while you are here fixing those groups, maybe fix this one as well?  At least, it would be great to have it uniformly corrected, so nobody would copy-and-paste this incorrect (I think) instance.


http://gerrit.cloudera.org:8080/#/c/16372/6/src/kudu/client/scan_batch.h
File src/kudu/client/scan_batch.h:

http://gerrit.cloudera.org:8080/#/c/16372/6/src/kudu/client/scan_batch.h@135
PS6, Line 135:   //
nit: maybe, correct this to be three slashes?



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 27 Aug 2020 07:01:49 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16372

to look at the new patch set (#5).

Change subject: Fix C++ client API docs
......................................................................

Fix C++ client API docs

This patch fixes C++ API doxygen comments so the build succeeds. It's
unclear when this broke exactly.

Running docs/support/scripts/make_site.sh with Doxygen 1.8.11 failed
with the below warning:

util/monotime.h:371: error: argument 'lhs' of command @param is not found in the argument list of kudu::MonoDelta::operator+(const MonoDelta &, const MonoDelta &) (warning treated as error, aborting now)

When Kudu 1.12.0 was released, the docs were generated with Doxygen
1.8.11, although it's possible that treating warnings as errors was
disabled for the release to work around this issue.

After upgrading to Doxygen 1.8.15 this warning disappeared, but a
different one surfaced:

common/partial_row.h:105: error: Member SetBool(const Slice &col_name, bool val) WARN_UNUSED_RESULT (function) of class KuduPartialRow is not documented. (warning treated as error, aborting now)

I disabled WARN_AS_ERROR temporarily only to discover several other
warnings. Some of them were due to Doxygen bugs solved by another update
to 1.8.19 in an earlier commit (change ID:
I5eddf689da90123ca52075d09de07981d11f8ffe), the rest manually by this
commit.

Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
---
M src/kudu/client/client.h
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema.h
M src/kudu/client/value.h
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/util/monotime.h
M src/kudu/util/status.h
9 files changed, 268 insertions(+), 161 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/16372/5
-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16372/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16372/3//COMMIT_MSG@29
PS3, Line 29: I5eddf689da90123ca52075d09de07981d11f8ffe
> nit: I guess it's better to use githash while referencing changelists
I agree, however, this change isn't committed yet, so the commit hash will still change.


http://gerrit.cloudera.org:8080/#/c/16372/2/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/16372/2/src/kudu/client/client.h@1173
PS2, Line 1173:   /// @warning Unstable API
              :   ///
> While you are at this, maybe move the method above under this Advanced/Unst
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 Aug 2020 18:13:35 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16372/2/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/16372/2/src/kudu/client/client.h@1173
PS2, Line 1173:   /// @warning Unstable API
              :   ///
While you are at this, maybe move the method above under this Advanced/Unstable API section as well?  Probably worth double-checking with Bankim, but as far as I can see the method above should belong to this section as well.



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 Aug 2020 18:03:25 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16372/1/src/kudu/client/scan_batch.h
File src/kudu/client/scan_batch.h:

http://gerrit.cloudera.org:8080/#/c/16372/1/src/kudu/client/scan_batch.h@a211
PS1, Line 211: 
> We lose a bit of "documentation" here given I think this was the only place
Done


http://gerrit.cloudera.org:8080/#/c/16372/1/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:

http://gerrit.cloudera.org:8080/#/c/16372/1/src/kudu/common/partial_row.h@a111
PS1, Line 111: 
             : 
             : 
             : 
> Same as my comment in scan_batch.
Done


http://gerrit.cloudera.org:8080/#/c/16372/1/src/kudu/common/partial_row.h@a452
PS1, Line 452: 
             : 
             : 
> Same as my comment in scan_batch.
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 Aug 2020 16:18:54 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16372/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16372/2//COMMIT_MSG@9
PS2, Line 9: the build succeeds
Thank you for fixing these doxygen issues.

Did you check the generated documentation w.r.t. to these changes?  Does everything look as expected?



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 Aug 2020 16:39:08 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16372

to look at the new patch set (#7).

Change subject: Fix C++ client API docs
......................................................................

Fix C++ client API docs

This patch fixes C++ API doxygen comments so the build succeeds. It's
unclear when this broke exactly.

Running docs/support/scripts/make_site.sh with Doxygen 1.8.11 failed
with the below warning:

util/monotime.h:371: error: argument 'lhs' of command @param is not found in the argument list of kudu::MonoDelta::operator+(const MonoDelta &, const MonoDelta &) (warning treated as error, aborting now)

When Kudu 1.12.0 was released, the docs were generated with Doxygen
1.8.11, although it's possible that treating warnings as errors was
disabled for the release to work around this issue.

After upgrading to Doxygen 1.8.15 this warning disappeared, but a
different one surfaced:

common/partial_row.h:105: error: Member SetBool(const Slice &col_name, bool val) WARN_UNUSED_RESULT (function) of class KuduPartialRow is not documented. (warning treated as error, aborting now)

I disabled WARN_AS_ERROR temporarily only to discover several other
warnings. Some of them were due to Doxygen bugs solved by another update
to 1.8.19 in commit de31237, the rest manually by this commit.

Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
---
M src/kudu/client/client.h
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema.h
M src/kudu/client/value.h
M src/kudu/common/partial_row.h
M src/kudu/util/monotime.h
M src/kudu/util/status.h
8 files changed, 264 insertions(+), 156 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/16372/7
-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 7
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 7:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16372/6/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/16372/6/src/kudu/client/client.h@1123
PS6, Line 1123: @warning This method is experimental and may change or disappear in futu
> nit: maybe it makes sense to make this and warning at line 1173 uniform, i.
Done


http://gerrit.cloudera.org:8080/#/c/16372/6/src/kudu/client/client.h@2438
PS6, Line 2438:   /// @name Advanced/Unstable API
              :   ///
              :   /// Modifier flags for the row format returned from the server.
              :   ///
              :   ///
> nit: while you are here fixing those groups, maybe fix this one as well?  A
Done


http://gerrit.cloudera.org:8080/#/c/16372/6/src/kudu/client/scan_batch.h
File src/kudu/client/scan_batch.h:

http://gerrit.cloudera.org:8080/#/c/16372/6/src/kudu/client/scan_batch.h@135
PS6, Line 135:   //
> nit: maybe, correct this to be three slashes?
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 7
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 27 Aug 2020 10:39:13 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16372/3/src/kudu/client/scan_batch.h
File src/kudu/client/scan_batch.h:

http://gerrit.cloudera.org:8080/#/c/16372/3/src/kudu/client/scan_batch.h@214
PS3, Line 214: @param [out] micros_since_utc_epoch
> here and below: I guess this should have been
Thanks, fixed it. I'd rather use the original parameter names as it makes it easier to understand what the output is as Grant pointed out.



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 Aug 2020 22:15:16 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16372/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16372/2//COMMIT_MSG@9
PS2, Line 9: the build succeeds
> Yes, I didn't see anything off.
Could you mention the version of doxygen or such in the commit message because I don't remember hitting these issues when I last ran make docs on a Linux machine (I haven't tried on a Mac OS)?



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 Aug 2020 17:08:37 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16372

to look at the new patch set (#4).

Change subject: Fix C++ client API docs
......................................................................

Fix C++ client API docs

This patch fixes C++ API doxygen comments so the build succeeds. It's
unclear when this broke exactly.

Running docs/support/scripts/make_site.sh with Doxygen 1.8.11 failed
with the below warning:

util/monotime.h:371: error: argument 'lhs' of command @param is not found in the argument list of kudu::MonoDelta::operator+(const MonoDelta &, const MonoDelta &) (warning treated as error, aborting now)

When Kudu 1.12.0 was released, the docs were generated with Doxygen
1.8.11, although it's possible that treating warnings as errors was
disabled for the release to work around this issue.

After upgrading to Doxygen 1.8.15 this warning disappeared, but a
different one surfaced:

common/partial_row.h:105: error: Member SetBool(const Slice &col_name, bool val) WARN_UNUSED_RESULT (function) of class KuduPartialRow is not documented. (warning treated as error, aborting now)

I disabled WARN_AS_ERROR temporarily only to discover several other
warnings. Some of them were due to Doxygen bugs solved by another update
to 1.8.19 in an earlier commit (change ID:
I5eddf689da90123ca52075d09de07981d11f8ffe), the rest manually by this
commit.

Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
---
M src/kudu/client/client.h
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema.h
M src/kudu/client/value.h
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/util/monotime.h
M src/kudu/util/status.h
9 files changed, 268 insertions(+), 161 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/16372/4
-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] Fix C++ client API docs

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16372 )

Change subject: Fix C++ client API docs
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16372/3/src/kudu/client/scan_batch.h
File src/kudu/client/scan_batch.h:

http://gerrit.cloudera.org:8080/#/c/16372/3/src/kudu/client/scan_batch.h@214
PS3, Line 214: @param [micros_since_utc_epoch] val
here and below: I guess this should have been

@param [out] micros_since_utc_epoch

?

I guess somewhat better alternative might be

@param [out] val
  Placeholder for the result value: microseconds since Epoch.



-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 Aug 2020 18:28:07 +0000
Gerrit-HasComments: Yes

[kudu-CR] Fix C++ client API docs

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16372

to look at the new patch set (#6).

Change subject: Fix C++ client API docs
......................................................................

Fix C++ client API docs

This patch fixes C++ API doxygen comments so the build succeeds. It's
unclear when this broke exactly.

Running docs/support/scripts/make_site.sh with Doxygen 1.8.11 failed
with the below warning:

util/monotime.h:371: error: argument 'lhs' of command @param is not found in the argument list of kudu::MonoDelta::operator+(const MonoDelta &, const MonoDelta &) (warning treated as error, aborting now)

When Kudu 1.12.0 was released, the docs were generated with Doxygen
1.8.11, although it's possible that treating warnings as errors was
disabled for the release to work around this issue.

After upgrading to Doxygen 1.8.15 this warning disappeared, but a
different one surfaced:

common/partial_row.h:105: error: Member SetBool(const Slice &col_name, bool val) WARN_UNUSED_RESULT (function) of class KuduPartialRow is not documented. (warning treated as error, aborting now)

I disabled WARN_AS_ERROR temporarily only to discover several other
warnings. Some of them were due to Doxygen bugs solved by another update
to 1.8.19 in an earlier commit (change ID:
I5eddf689da90123ca52075d09de07981d11f8ffe), the rest manually by this
commit.

Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
---
M src/kudu/client/client.h
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema.h
M src/kudu/client/value.h
M src/kudu/common/partial_row.h
M src/kudu/util/monotime.h
M src/kudu/util/status.h
8 files changed, 259 insertions(+), 151 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/16372/6
-- 
To view, visit http://gerrit.cloudera.org:8080/16372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I387be394d656a7614189148a39ff7dec76eb994d
Gerrit-Change-Number: 16372
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)