Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

doc: remove duplicate definitions of Doxygen groups #11814

Merged
merged 12 commits into from
Aug 6, 2019
Merged

doc: remove duplicate definitions of Doxygen groups #11814

merged 12 commits into from
Aug 6, 2019

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Jul 7, 2019 •
edited
Loading

Contribution description

This PR is fixing all duplicate definitions of Doxygen groups reported by the doccheck script PRed in #11813.

The last commit of this PR is updating the script so that it returns an error when duplicate Doxygen group definitions are reported by the script.

Testing procedure

  • A green CI: make static-test returns with no error
  • Doxygen documentation is correct: run make doc and verify

Issues/PRs references

Based on #11813 but can also be merged directly.

@aabadie aabadie added Area: doc Area: Documentation Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Jul 7, 2019
@aabadie aabadie requested review from cladmi and miri64 July 7, 2019 16:19
@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 7, 2019
@aabadie aabadie mentioned this pull request Jul 7, 2019
Merged
@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 30, 2019
miri64
miri64 requested changes Jul 30, 2019
View reviewed changes
Copy link
Member

@miri64 miri64 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested that make doc still works. However, I find a minor difference.

pkg/lwip/include/lwip.h
@@ -7,13 +7,7 @@
*/

/**
* @defgroup pkg_lwip lwIP
* @ingroup pkg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is removed without replacement, causing lwIP not to show up under pkg anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already the case in master (and it's related to #8664).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right

@aabadie
Copy link
Contributor Author

aabadie commented Aug 5, 2019

I find a minor difference.

Can you point to that difference ? For the moment, I can't any reference to lwip on doc.riot-os.org.

@miri64
Copy link
Member

miri64 commented Aug 5, 2019

I find a minor difference.

Can you point to that difference ? For the moment, I can't any reference to lwip on doc.riot-os.org.

Mh... maybe I've mis-seen something, because now I can confirm that

miri64
miri64 previously approved these changes Aug 5, 2019
View reviewed changes
pkg/lwip/include/lwip.h
@@ -7,13 +7,7 @@
*/

/**
* @defgroup pkg_lwip lwIP
* @ingroup pkg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right

@miri64
Copy link
Member

miri64 commented Aug 5, 2019

Can you remove 7b0ac83 so we can do as suggested in op?

but can also be merged directly.

@miri64
Copy link
Member

miri64 commented Aug 5, 2019

(I did not review the bash code yet and it is still under review in #11813.

@miri64 miri64 requested review from miri64 and removed request for cladmi August 5, 2019 13:51
@miri64 miri64 dismissed their stale review August 5, 2019 13:52

Waiting for 8b0ac83 to be removed

@miri64
Copy link
Member

miri64 commented Aug 5, 2019

I meant 7b0ac83... (can't edit the post)

@aabadie
Copy link
Contributor Author

aabadie commented Aug 5, 2019

Thanks for having a look @miri64. Is it ok if I cut out 7b0ac83 and 0b46644 and apply the latter to #11813 directly ?

@miri64
Copy link
Member

miri64 commented Aug 5, 2019

Yepp of course!

aabadie added 10 commits August 5, 2019 16:57
@aabadie
boards/esp8266: deduplicate board doxygen group definition
248f8d3
@aabadie
boards/nucleo-*: deduplicate doxygen board group definitions
01814de
@aabadie
boards/stm32f769i-disco: deduplicate doxygen group definition
6bf9290 
The group is already defined in the doc.txt file so no need to define it again in board.h
@aabadie
cpu/efm32: deduplicate cpu_efm32 group definition
570f308
@aabadie
sys/sha1: deduplicate sys_hashes_sha1 group definition
9e9ef32
@aabadie
sys/pthread: deduplicate pthread group definition
998fd6b
@aabadie
pkg/lwip: cleanup doxygen documentation
bd816f3 
- Deduplicate lwip group definition
- Fix reference to sock instead of conn
@aabadie
boards/common/esp: deduplicate common esp boards group definitions
90e1364
@aabadie
boards/arduino-leonardo: remove duplicate doc group definition
8196ea7
@aabadie
core/sched_native: fix duplicate core_sched group definition
fdfd313 
Introduce core_sched_native group and put this in group in parent
core_sched group
@aabadie aabadie force-pushed the pr/doc/remove_duplicate_group_define branch from 0b46644 to e352b54 Compare August 5, 2019 14:57
@aabadie
Copy link
Contributor Author

aabadie commented Aug 5, 2019

Done :)

@miri64
Copy link
Member

miri64 commented Aug 5, 2019

Since you also rebased to master, I will need to look into this again in detail. I'll do so tomorrow.

@miri64
Copy link
Member

miri64 commented Aug 6, 2019

Range diff is okay:

$ git range-diff upstream/master 0b46644 HEAD
 1:  7b0ac83ac9 <  -:  ---------- tools/doccheck: add multiple group definition check
 2:  fb738cc272 =  1:  248f8d3284 boards/esp8266: deduplicate board doxygen group definition
 3:  814445c3cd =  2:  01814dec19 boards/nucleo-*: deduplicate doxygen board group definitions
 4:  dfa07f2dfd =  3:  6bf9290ed5 boards/stm32f769i-disco: deduplicate doxygen group definition
 5:  746174be71 =  4:  570f308551 cpu/efm32: deduplicate cpu_efm32 group definition
 6:  05a0b8ef4e =  5:  9e9ef3293d sys/sha1: deduplicate sys_hashes_sha1 group definition
 7:  34dbaa92ff =  6:  998fd6bcc9 sys/pthread: deduplicate pthread group definition
 8:  6be82342fd =  7:  bd816f3fb1 pkg/lwip: cleanup doxygen documentation
 9:  1411196e60 =  8:  90e136405e boards/common/esp: deduplicate common esp boards group definitions
10:  74b68e61ff =  9:  8196ea7e58 boards/arduino-leonardo: remove duplicate doc group definition
11:  ce0a9320e6 = 10:  fdfd3133f7 core/sched_native: fix duplicate core_sched group definition
12:  db32d467cb = 11:  f922df8c99 net/gnrc/sock: remove duplicate net_gnrc_sock group definition
13:  0c99f9f5b8 = 12:  e352b54a9b net/gnrc/tcp: remove duplicate net_gnrc_tcp group definitions
14:  0b46644f23 <  -:  ---------- tools/doccheck: enable error for multiple group defines

@miri64 miri64 merged commit d13d49d into RIOT-OS:master Aug 6, 2019
@aabadie aabadie deleted the pr/doc/remove_duplicate_group_define branch August 6, 2019 07:53
@aabadie
Copy link
Contributor Author

aabadie commented Aug 6, 2019

Thanks @miri64 !

@kb2ma kb2ma added this to the Release 2019.10 milestone Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miri64 miri64 miri64 approved these changes

Assignees
No one assigned
Labels
Area: doc Area: Documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Milestone
Release 2019.10
Development

Successfully merging this pull request may close these issues.
3 participants
@aabadie @miri64 @kb2ma