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.

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

[travis] Fail when documentation is outdated #7280

Merged
merged 2 commits into from
Jan 19, 2016

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

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.

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

Choose a reason for hiding this comment

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

Copy link
Member

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 notifications@github.com 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.

REGEX_DOC = re.compile(r'HelpMessageOpt\(\"(\-[^\"=]+?)(?:=|\")')

def main():
used = check_output(CMD_GREP_ARGS, shell=True)
Copy link
Member

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

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
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
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
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
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.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants