Update doxygen config by karczex · Pull Request #1177 · pmem/libpmemobj-cpp

karczex · 2021-07-27T12:45:16Z

This PR is build on top of #1074 with some output tweaks.

This change is

igchor

Can we change the "Module" into "Group" or something similar? Usually, module means some standalone package/unit and not a collection of different classes.

Reviewable status: 0 of 37 files reviewed, 1 unresolved discussion (waiting on @karczex)

a discussion (no related file):
Regarding modules' names, I would suggest following change:

Data residency -> Allocation
Data accessors -> Data views
Split primitives into primitives and synchronization

Also, some things are missing I think (e.g pool, ctl)

lukaszstolarczuk

Reviewable status: 0 of 37 files reviewed, 5 unresolved discussions (waiting on @igchor and @karczex)

a discussion (no related file):
pls update CONTRIBUTING.md and mention docs grouping (@ingroup)

a discussion (no related file):
pls add ref (in some commit msg) to #1010

a discussion (no related file):
misspell in commit message Increarease -> Increase

a discussion (no related file):

Previously, igchor (Igor Chorążewicz) wrote…

Regarding modules' names, I would suggest following change:

Data residency -> Allocation
Data accessors -> Data views
Split primitives into primitives and synchronization

Also, some things are missing I think (e.g pool, ctl)

do we want to have everything grouped? or just "the most important" classes...?

I guess, all public API should be assigned to some groups (so I think I agree with Igor 😉 )

doc/libpmemobj++.Doxyfile.in, line 872 at r2 (raw file):

INPUT                  = @LIBPMEMOBJCPP_ROOT_DIR@/include/libpmemobj++ \
                         @LIBPMEMOBJCPP_ROOT_DIR@/include/libpmemobj++/README.md \
                         @LIBPMEMOBJCPP_ROOT_DIR@/doc/group_defs.dox

can you deliver at least a stub of this file (this '.dox' file perhaps requires some special syntax...?)
and please rename to e.g. groups_definitions

codecov · 2021-07-27T13:35:30Z

Codecov Report

Merging #1177 (a788092) into master (14677b5) will decrease coverage by 0.54%.
The diff coverage is 37.50%.

❗ Current head a788092 differs from pull request most recent head c231064. Consider uploading reports for the commit c231064 to get more accurate results

@@            Coverage Diff             @@
##           master    #1177      +/-   ##
==========================================
- Coverage   94.12%   93.58%   -0.55%     
==========================================
  Files          52       52              
  Lines        5144     4442     -702     
==========================================
- Hits         4842     4157     -685     
+ Misses        302      285      -17

Flag	Coverage Δ
tests_clang_debug_cpp17	`93.18% <37.50%> (-0.52%)`	⬇️
tests_gcc_debug	`90.09% <36.36%> (-0.65%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
include/libpmemobj++/allocator.hpp	`75.86% <ø> (+4.80%)`	⬆️
include/libpmemobj++/condition_variable.hpp	`70.83% <ø> (-7.74%)`	⬇️
include/libpmemobj++/container/array.hpp	`97.33% <ø> (-0.75%)`	⬇️
include/libpmemobj++/container/basic_string.hpp	`100.00% <ø> (+0.98%)`	⬆️
...j++/container/detail/concurrent_skip_list_impl.hpp	`96.69% <ø> (-0.31%)`	⬇️
include/libpmemobj++/container/segment_vector.hpp	`92.85% <ø> (-1.27%)`	⬇️
include/libpmemobj++/detail/ctl.hpp	`100.00% <ø> (ø)`
include/libpmemobj++/detail/pair.hpp	`100.00% <ø> (ø)`
include/libpmemobj++/detail/tagged_ptr.hpp	`100.00% <ø> (ø)`
include/libpmemobj++/detail/temp_value.hpp	`100.00% <ø> (ø)`
... and 54 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14677b5...c231064. Read the comment docs.

KFilipek

Reviewed 1 of 16 files at r1, 1 of 28 files at r2.
Reviewable status: 2 of 37 files reviewed, 5 unresolved discussions (waiting on @igchor and @karczex)

KFilipek

Reviewable status: 2 of 37 files reviewed, 6 unresolved discussions (waiting on @igchor and @karczex)

doc/libpmemobj++.Doxyfile.in, line 494 at r2 (raw file):

EXTRACT_PRIVATE        = YES

I think it should be disabled, to avoid generating documentation for garbage like helpers methods in string, output there is massive

karczex

Reviewable status: 1 of 43 files reviewed, 6 unresolved discussions (waiting on @igchor, @KFilipek, and @lukaszstolarczuk)

a discussion (no related file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

pls update CONTRIBUTING.md and mention docs grouping (@ingroup)

Done.

a discussion (no related file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

pls add ref (in some commit msg) to #1010

Done.

a discussion (no related file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

misspell in commit message Increarease -> Increase

Done.

doc/libpmemobj++.Doxyfile.in, line 494 at r2 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

EXTRACT_PRIVATE        = YES
I think it should be disabled, to avoid generating documentation for garbage like helpers methods in string, output there is massive

Done.

doc/libpmemobj++.Doxyfile.in, line 872 at r2 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

can you deliver at least a stub of this file (this '.dox' file perhaps requires some special syntax...?)

and please rename to e.g. groups_definitions

Done.

igchor

Reviewed 6 of 16 files at r1, 33 of 35 files at r4.
Reviewable status: 39 of 43 files reviewed, 5 unresolved discussions (waiting on @KFilipek and @lukaszstolarczuk)

doc/libpmemobj++.Doxyfile.in, line 842 at r5 (raw file):

# or unneeded files are turned off (from doxygen)
#WARN_AS_ERROR          = @DOXYGEN_WARN_AS_ERROR@
WARN_AS_ERROR          = NO

Could you create an issue for this (unless we already have one)?

KFilipek

Reviewed 7 of 16 files at r1, 1 of 28 files at r2, 35 of 35 files at r4.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @karczex and @lukaszstolarczuk)

include/libpmemobj++/timed_mutex.hpp, line 31 at r5 (raw file):

 * concepts. The typical usage example would be:
 * @snippet mutex/mutex.cpp timed_mutex_example
 * @ingorup primitives

@InGroup

include/libpmemobj++/container/segment_vector.hpp, line 661 at r5 (raw file):

/* Non-member swap
 * @relates segment_vector
 * */

replace

 * */

with

*/

include/libpmemobj++/container/vector.hpp, line 258 at r5 (raw file):

/* Non-member swap
 * @relates vector
 * */

same story

include/libpmemobj++/detail/temp_value.hpp, line 37 at r5 (raw file):

struct temp_value;

/*

If it should be ignored by Doxygen just skip my comment

karczex

Reviewable status: 40 of 43 files reviewed, 7 unresolved discussions (waiting on @igchor, @KFilipek, and @lukaszstolarczuk)

doc/libpmemobj++.Doxyfile.in, line 842 at r5 (raw file):

Previously, igchor (Igor Chorążewicz) wrote…

Could you create an issue for this (unless we already have one)?

#1182

include/libpmemobj++/timed_mutex.hpp, line 31 at r5 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

@InGroup

Good catch. Done

include/libpmemobj++/container/segment_vector.hpp, line 661 at r5 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

replace
 * */
with
*/

Done.

include/libpmemobj++/container/vector.hpp, line 258 at r5 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

same story

Done.

KFilipek · 2021-07-28T13:36:02Z

include/libpmemobj++/make_persistent_array.hpp:1: error: wrong copyright date: (is: 2016-2020, should be: 2016-2021)
include/libpmemobj++/make_persistent_atomic.hpp:1: error: wrong copyright date: (is: 2016-2020, should be: 2016-2021)

lukaszstolarczuk

Reviewed 8 of 16 files at r1, 30 of 35 files at r4, 3 of 3 files at r6.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @karczex and @KFilipek)

CONTRIBUTING.md, line 40 at r6 (raw file):

* Each new feature should be documented using doxygen comment blocks.
* Each non-member function should be added to corresponding class documentation by `@relates` tag
* Each new API should be assigned to group by `@ingroup` tag. See `doc/groups_definitions.dox` file in the repository for details.

you can add a link to the file, actually

doc/groups_definitions.dox, line 2 at r6 (raw file):

# SPDX-License-Identifier: BSD-3-Clause
# Copyright 2018-2021, Intel Corporation

2021 should be enough

include/libpmemobj++/pool.hpp, line 49 at r6 (raw file):

 * The exmple of using pool with RAII idiom:
 * @snippet pool/pool_as_class_member.cpp  pool_class_member_example
 *

why the empty line? Did you want to add something here? perhaps it should be in the primitives group (since it's a public class)

include/libpmemobj++/detail/temp_value.hpp, line 37 at r5 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

If it should be ignored by Doxygen just skip my comment

If I remember correctly, this comment made our lives harder and we decided to disable it in Doxygen parsing... and it's just a detail ns.

include/libpmemobj++/experimental/atomic_self_relative_ptr.hpp, line 26 at r6 (raw file):

 * Doesn't automatically add itself to the transaction.
 * The user is responsible for persisting the data.
 *

remove this empty line (?)

utils/docker/run-doc-update.sh, line 50 at r6 (raw file):

pushd ${REPO_DIR}
echo "Clone repo:"
git clone --origin Local ${LOCAL_REPO} ${REPO_DIR}

lowercase local, for consistency

utils/docker/run-doc-update.sh, line 52 at r6 (raw file):

git clone --origin Local ${LOCAL_REPO} ${REPO_DIR}
cd ${REPO_DIR}
git remote add origin ${UPSTREAM}

UPSTREAM -> ORIGIN

utils/docker/run-doc-update.sh, line 60 at r6 (raw file):

git remote update
git checkout -B ${TARGET_BRANCH} Local/${TARGET_BRANCH}

does this change work for you? if you're not on master/stable branch this script should exit (in the if section on top of the file)

it has doxygen in version 1.9.x, which produces better documentation. Also, removed additional 'hub --version' step in the auto-doc-update script, since it appears to be redundant. Fixes: pmem#1010

using Doxygen 1.9.1 version.

Include graph is extremely unreadable, and not really useful

Fixes pmem#1076

It's needed for ebr_0_drd and radix_concurrent_ierate_0_drd

It's needed to rename "Modules" section into "Features" https://www.doxygen.nl/manual/customize.html

It generates a lot not relevant documentation for helper methods

karczex

Reviewable status: 8 of 43 files reviewed, 10 unresolved discussions (waiting on @igchor, @KFilipek, and @lukaszstolarczuk)

CONTRIBUTING.md, line 40 at r6 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

you can add a link to the file, actually

Done.

doc/groups_definitions.dox, line 2 at r6 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

2021 should be enough

Done.

include/libpmemobj++/pool.hpp, line 49 at r6 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

why the empty line? Did you want to add something here? perhaps it should be in the primitives group (since it's a public class)

Done.

include/libpmemobj++/experimental/atomic_self_relative_ptr.hpp, line 26 at r6 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

remove this empty line (?)

Done.

utils/docker/run-doc-update.sh, line 50 at r6 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

lowercase local, for consistency

:)
It cannot be done consistently. On one hand local is keyword in bash, on the other hand "local" is a word, which is commonly used to describe branch like this one.

utils/docker/run-doc-update.sh, line 52 at r6 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

UPSTREAM -> ORIGIN

Done.

utils/docker/run-doc-update.sh, line 60 at r6 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

does this change work for you? if you're not on master/stable branch this script should exit (in the if section on top of the file)

It works, as I'm doing changes on my local master. I don't want to change basic functionality of this script, rather make it little bit more usable.

lukaszstolarczuk

Reviewed 1 of 35 files at r9, 32 of 34 files at r10, 1 of 1 files at r11.
Reviewable status: 42 of 43 files reviewed, 3 unresolved discussions (waiting on @igchor and @KFilipek)

utils/docker/run-doc-update.sh, line 50 at r6 (raw file):

Previously, karczex (Paweł Karczewski) wrote…

:)
It cannot be done consistently. On one hand local is keyword in bash, on the other hand "local" is a word, which is commonly used to describe branch like this one.

och, yeah, my bad, I forgot it's a keyword

utils/docker/run-doc-update.sh, line 60 at r6 (raw file):

Previously, karczex (Paweł Karczewski) wrote…

It works, as I'm doing changes on my local master. I don't want to change basic functionality of this script, rather make it little bit more usable.

ok, sure, if you make it on master then everything is clear to me

karczex

Reviewable status: 42 of 43 files reviewed, 3 unresolved discussions (waiting on @igchor and @KFilipek)

include/libpmemobj++/timed_mutex.hpp, line 31 at r5 (raw file):

Previously, karczex (Paweł Karczewski) wrote…

Good catch. Done

Done.

include/libpmemobj++/container/segment_vector.hpp, line 661 at r5 (raw file):

Previously, karczex (Paweł Karczewski) wrote…

Done.

Done.

KFilipek

Reviewable status: 42 of 43 files reviewed, all discussions resolved (waiting on @igchor and @KFilipek)

igchor suggested changes Jul 27, 2021

View reviewed changes

lukaszstolarczuk reviewed Jul 27, 2021

View reviewed changes

KFilipek reviewed Jul 28, 2021

View reviewed changes

karczex force-pushed the update-doxygen-config branch 2 times, most recently from 8a507cd to eda212c Compare July 28, 2021 11:26

karczex commented Jul 28, 2021

View reviewed changes

igchor approved these changes Jul 28, 2021

View reviewed changes

KFilipek reviewed Jul 28, 2021

View reviewed changes

karczex force-pushed the update-doxygen-config branch 2 times, most recently from ab8d000 to 5b12f9f Compare July 28, 2021 13:21

karczex commented Jul 28, 2021

View reviewed changes

lukaszstolarczuk reviewed Jul 28, 2021

View reviewed changes

karczex force-pushed the update-doxygen-config branch from 5b12f9f to 80295bb Compare July 28, 2021 14:37

lukaszstolarczuk and others added 6 commits July 29, 2021 06:46

utils: run auto documentation updates on Fedora 34

2b1abc2

it has doxygen in version 1.9.x, which produces better documentation. Also, removed additional 'hub --version' step in the auto-doc-update script, since it appears to be redundant. Fixes: pmem#1010

doc: update Doxygen config file to new format

0e8dd0c

using Doxygen 1.9.1 version.

doc: enable additional Doxygen options for generating better docs

6024982

doc: update doxygen comments and documentation

bd08177

[doc] Disable include graphs

9ec6394

Include graph is extremely unreadable, and not really useful

[doc] Add non-member methods documentation to corresponding pages

aa94336

Fixes pmem#1076

karczex force-pushed the update-doxygen-config branch from 80295bb to a788092 Compare July 29, 2021 04:46

Pawel Karczewski added 5 commits July 29, 2021 06:47

[doc] Add Modules page to doxgen documentation

d191a1a

Increase test timeout

e2ef2e1

It's needed for ebr_0_drd and radix_concurrent_ierate_0_drd

[doc] Introduce doxygen layout file

4b9a481

It's needed to rename "Modules" section into "Features" https://www.doxygen.nl/manual/customize.html

[doc] Enable uploading documentation for local builds

c9ddd3b

[doc] Disable EXTRACT_PRIVATE

c231064

It generates a lot not relevant documentation for helper methods

karczex force-pushed the update-doxygen-config branch from a788092 to c231064 Compare July 29, 2021 04:47

karczex commented Jul 29, 2021

View reviewed changes

lukaszstolarczuk reviewed Jul 29, 2021

View reviewed changes

karczex commented Jul 29, 2021

View reviewed changes

KFilipek reviewed Jul 29, 2021

View reviewed changes

igchor merged commit e8f5621 into pmem:master Jul 29, 2021

lukaszstolarczuk mentioned this pull request Jan 4, 2022

fix a doxygen warning pmem/pmemkv#1055

Merged

Conversation

karczex commented Jul 27, 2021 • edited by igchor Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

igchor left a comment

Choose a reason for hiding this comment

Uh oh!

lukaszstolarczuk left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jul 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

KFilipek left a comment

Choose a reason for hiding this comment

Uh oh!

KFilipek left a comment

Choose a reason for hiding this comment

Uh oh!

karczex left a comment

Choose a reason for hiding this comment

Uh oh!

igchor left a comment

Choose a reason for hiding this comment

Uh oh!

KFilipek left a comment

Choose a reason for hiding this comment

Uh oh!

karczex left a comment

Choose a reason for hiding this comment

Uh oh!

KFilipek commented Jul 28, 2021

Uh oh!

lukaszstolarczuk left a comment

Choose a reason for hiding this comment

Uh oh!

karczex left a comment

Choose a reason for hiding this comment

Uh oh!

lukaszstolarczuk left a comment

Choose a reason for hiding this comment

Uh oh!

karczex left a comment

Choose a reason for hiding this comment

Uh oh!

KFilipek left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

karczex commented Jul 27, 2021 •

edited by igchor

Loading

codecov bot commented Jul 27, 2021 •

edited

Loading