Skip to content

Run flake8 tests on all new code submissions#619

Closed
cclauss wants to merge 1 commit intobitcoin:masterfrom
cclauss:patch-1
Closed

Run flake8 tests on all new code submissions#619
cclauss wants to merge 1 commit intobitcoin:masterfrom
cclauss:patch-1

Conversation

@cclauss
Copy link
Copy Markdown

@cclauss cclauss commented Dec 25, 2017

flake8 testing of https://github.com/bitcoin/bips on Python 3.6.3

$ flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics

./bip-0069/bip-0069_examples.py:52:9: F821 undefined name 'bytearray_cmp'
	return bytearray_cmp(output_tuple1[1], output_tuple2[1])
               ^
1     F821 undefined name 'bytearray_cmp'

flake8 testing of https://github.com/bitcoin/bips on Python 3.6.3

$ __flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics__
```
./bip-0069/bip-0069_examples.py:52:9: F821 undefined name 'bytearray_cmp'
	return bytearray_cmp(output_tuple1[1], output_tuple2[1])
               ^
1     F821 undefined name 'bytearray_cmp'
```
@luke-jr
Copy link
Copy Markdown
Member

luke-jr commented Dec 25, 2017

Why? BIPs aren't source code, they're documents...

@cclauss
Copy link
Copy Markdown
Author

cclauss commented Dec 25, 2017

Because this repo contains ./bip-0069/bip-0069_examples.py and that file could raise a NameError exception at runtime. How can users verify the validity of implementations when the examples crash?

See: https://travis-ci.org/bitcoin/bips/builds/321281686

@promag
Copy link
Copy Markdown

promag commented May 23, 2018

@luke-jr why not? @cclauss maybe include the fix in a commit (before the current commit)?

@cclauss cclauss closed this Sep 22, 2019
@cclauss cclauss deleted the patch-1 branch September 22, 2019 16:50
cclauss added a commit to cclauss/bips that referenced this pull request Jan 23, 2020
As discussed in bitcoin#619.  `bytearray_cmp` is and undefined name in this context which has the potential to raise NameError at runtime so use `bytearr_cmp()` (which is defined on line 4) instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants