Skip to content

[travis] Fail when documentation is outdated#7280

Merged
laanwj merged 2 commits intobitcoin:masterfrom
maflcko:MarcoFalke-2015-travisDoc
Jan 19, 2016
Merged

[travis] Fail when documentation is outdated#7280
laanwj merged 2 commits intobitcoin:masterfrom
maflcko:MarcoFalke-2015-travisDoc

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Jan 4, 2016

This checks if all command line args are documented in the help message.

Example output:

$ qa/pull-tester/check-doc.py 
Args used        : 144
Args documented  : 133
Args undocumented: 15
set(['-uacomment', '-nodebug', '-rpcssl', '-rpccookiefile', '-checkblockindex', '-benchmark', '-help', '-version', '-h', '-checkmempool', '-mocktime', '-maptxfee', '-tor', '-socks', '-debugnet'])
Args unknown     : 4
set(['-zmqpubrawblock', '-zmqpubhashtx', '-zmqpubrawtx', '-zmqpubhashblock'])
$ echo $?
15

@dcousens
Copy link
Contributor

dcousens commented Jan 4, 2016

concept ACK

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Jan 4, 2016

Nice!
We just need to be clear: this would mean, we extend the CI/pull-tester from functional code testing to style/documentation quality assurance.

IMO, this is something we should do.

utACK fa0f5100493572ea7abd2b0a9d98c697adc31014.

jonasschnelli
jonasschnelli reviewed Jan 4, 2016
View reviewed changes
qa/pull-tester/check-doc.py Outdated
Copy link
Contributor

@jonasschnelli jonasschnelli Jan 4, 2016

Choose a reason for hiding this comment

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

nit: add copyright header (https://github.com/MarcoFalke/bitcoin/blob/MarcoFalke-2015-travisDoc/qa/pull-tester/rpc-tests.py#L2)

Copy link
Member

@fanquake fanquake Jan 4, 2016

Choose a reason for hiding this comment

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

Concept ACK

On Monday, 4 January 2016, Jonas Schnelli [email protected] wrote:

In qa/pull-tester/check-doc.py
#7280 (comment):

@@ -0,0 +1,39 @@
+#!/usr/bin/env python
+'''

nit: add copyright header (
https://github.com/MarcoFalke/bitcoin/blob/MarcoFalke-2015-travisDoc/qa/pull-tester/rpc-tests.py#L2
)

—
Reply to this email directly or view it on GitHub
https://github.com/bitcoin/bitcoin/pull/7280/files#r48710140.

laanwj
laanwj reviewed Jan 4, 2016
View reviewed changes
qa/pull-tester/check-doc.py Outdated
Copy link
Member

@laanwj laanwj Jan 4, 2016

Choose a reason for hiding this comment

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

I'd prefer to never use any of the subprocess commands with shell=True, using a shell can easily lead to security issues.

Copy link
Member Author

@maflcko maflcko Jan 4, 2016

Choose a reason for hiding this comment

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

I agree that shell=True should never be used. But I am just too lazy to translate the pipe into python. The CMD_s are hardcoded, so shell injection is rather unlikely to happen.

@laanwj
Copy link
Member

laanwj commented Jan 4, 2016

Concept ACK
It's quite ugly to do this as long as there is no organization to the argument parsing, but as long as the risk of false positives is low it's good to add the check.

@maflcko maflcko force-pushed the MarcoFalke-2015-travisDoc branch from fa0f510 to 7497b86 Compare January 4, 2016 18:20
@maflcko maflcko force-pushed the MarcoFalke-2015-travisDoc branch from 7497b86 to fada0c9 Compare January 4, 2016 18:24
@maflcko
Copy link
Member Author

maflcko commented Jan 4, 2016

Moved check-doc.py to devtools and addressed nit by @jonasschnelli

@laanwj
Copy link
Member

laanwj commented Jan 7, 2016

Travis:

/home/travis/build.sh: line 45: contrib/devtools/check-doc.py: No such file or directory

@maflcko maflcko force-pushed the MarcoFalke-2015-travisDoc branch from 128e7f0 to a1c905c Compare January 7, 2016 15:15
@maflcko maflcko force-pushed the MarcoFalke-2015-travisDoc branch from a1c905c to faeda0e Compare January 18, 2016 13:01
@maflcko
Copy link
Member Author

maflcko commented Jan 18, 2016

Documentation was added in master, so travis should pass now.

@laanwj laanwj merged commit faeda0e into bitcoin:master Jan 19, 2016
laanwj added a commit that referenced this pull request Jan 19, 2016
@laanwj
Merge pull request #7280
668906f 
faeda0e [travis] Run contrib/devtools/check-doc.py early (MarcoFalke)
fada0c9 [travis] Fail when documentation is outdated (MarcoFalke)
@maflcko maflcko deleted the MarcoFalke-2015-travisDoc branch January 19, 2016 15:52
codablock pushed a commit to codablock/dash that referenced this pull request Dec 9, 2017
@laanwj @codablock
Merge pull request bitcoin#7280
42997ee 
faeda0e [travis] Run contrib/devtools/check-doc.py early (MarcoFalke)
fada0c9 [travis] Fail when documentation is outdated (MarcoFalke)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@maflcko @dcousens @jonasschnelli @laanwj @fanquake