Skip to content

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Oct 11, 2018

Contribution description

Removes doc group checks from dist/tools. Doxygen is not supposed to build that so checking it doesn't make sense.

Testing procedure

Take e.g. #10072 and run this fix on it. In current state (0ca02de & 19d0da0) it should fail in current master. It is fixed with this PR.

Issues/PRs references

See #10072 (comment)

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: doc Area: Documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: tools Area: Supplementary tools labels Oct 11, 2018
@miri64 miri64 requested a review from aabadie October 11, 2018 13:10
@@ -32,7 +32,7 @@ then
fi

exclude_filter() {
grep -v -e vendor -e examples -e tests
grep -e vendor -e examples -e tests -e "\<dist/tools\>"
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove -v ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Leftover from my experimentations with this script. I noticed it already when retesting, but forgot to put it back.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed and squashed

@miri64 miri64 force-pushed the tools/fix/doccheck branch from 38844c9 to 6f8fd1d Compare October 11, 2018 13:16
@@ -32,7 +32,7 @@ then
fi

exclude_filter() {
grep -v -e vendor -e examples -e tests
grep -v -e vendor -e examples -e tests -e "\<dist/tools\>"
Copy link
Contributor

Choose a reason for hiding this comment

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

second question: why using "\< \>". Note that it also works with just dist/tools

Copy link
Member Author

Choose a reason for hiding this comment

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

to exclude corner cases like e.g. absurdist/toolspinner ;-) might also be a good idea to adapt this for the other pattrens if applicable. But that's a task for a different PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

ACK

@miri64
Copy link
Member Author

miri64 commented Oct 11, 2018

Codacy takes longer than Murdock for a one-line change o_O

@aabadie
Copy link
Contributor

aabadie commented Oct 11, 2018

Codacy is done, let's go.

@aabadie aabadie merged commit 26abf4c into RIOT-OS:master Oct 11, 2018
@miri64 miri64 deleted the tools/fix/doccheck branch October 11, 2018 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: doc Area: Documentation 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 Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants