Skip to content
New issue

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

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

Already on GitHub? # to your account

tools/doccheck: add simple exclude to doccheck #19240

Merged
merged 1 commit into from
Feb 7, 2023

Conversation

kfessel
Copy link
Contributor

@kfessel kfessel commented Feb 3, 2023

while doccheck runs for #19228 and #19220, i saw some spikes in memory consumption, turned out that was grep -Evf dist/tools/doccheck/exclude_patterns using about 2GB RAM. This PR changes that.

Contribution description

add exclude_simple to doccheck drived from exclude patterns
sorted and uniqued the simple excludes
removes no longer needed patterns from exclude patterns

simple excludes are string rules (no patterns just strings)
how to apply these:
in this PR:
*remove the path and line number from the rule

  • that made some of them doubles of each other
  • sorted and uniqued them.
  • this set of excludes is no longer path specific (an exception covers all paths but may of them still contain a file name)

another possible solution would be to have the excludes line number specific.

Testing procedure

run dist/tools/doccheck/check.sh

compare memory consumption of
master: grep -Evf dist/tools/doccheck/exclude_patterns
to
this PR: grep -Fvf dist/tools/doccheck/exclude_simple

Issues/PRs references

@github-actions github-actions bot added the Area: tools Area: Supplementary tools label Feb 3, 2023
@kfessel kfessel requested a review from maribu February 3, 2023 16:18
@kfessel kfessel added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 3, 2023
@riot-ci
Copy link

riot-ci commented Feb 3, 2023

Murdock results

✔️ PASSED

ce6a42e tools/doccheck: add simple exclude to doccheck

Success Failures Total Runtime
1 0 1 59s

Artifacts

Comment on lines 1 to 2
drivers/ir_nec/include/ir_nec_params.h:35: warning: Member IR_NEC_PARAM_PIN (macro definition) of file ir_nec_params.h is not documented.
drivers/ir_nec/include/ir_nec_params.h:39: warning: Member IR_NEC_PARAMS (macro definition) of file ir_nec_params.h is not documented.
Copy link
Member

Choose a reason for hiding this comment

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

These two actually need a regex to match the line numbers, otherwise anyone adding or deleting a line above the definition of the macros would need to touch this file again.

Copy link
Contributor Author

@kfessel kfessel Feb 6, 2023

Choose a reason for hiding this comment

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

right (they where like this in the exclude_patterns before)

! there are no regex in exclude_simple that is the reason it requires less memory !

Copy link
Member

Choose a reason for hiding this comment

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

Oops :(

Care to piggy-back a fix?

Copy link
Contributor Author

@kfessel kfessel Feb 6, 2023

Choose a reason for hiding this comment

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

i made that a exclude simple line

warning: Member IR_NEC_PARAM_PIN (macro definition) of file ir_nec_params.h is not documented.
warning: Member IR_NEC_PARAMS (macro definition) of file ir_nec_params.h is not documented.

see the fixup

uniq and sorted simple excludes
removes no longer needed exclude patterns
@benpicco benpicco requested a review from kaspar030 February 6, 2023 16:01
@benpicco
Copy link
Contributor

benpicco commented Feb 7, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 7, 2023

Build succeeded:

@bors bors bot merged commit dd2d336 into RIOT-OS:master Feb 7, 2023
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants