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

Tools: improve verify-commits.py script #14809

Merged
merged 1 commit into from Dec 10, 2018

Conversation

Projects
None yet
8 participants
@jlopp
Copy link
Contributor

jlopp commented Nov 26, 2018

I ran into 3 different issues while trying to run the verify-commits script for the first time and I think documenting them would help save time for future developers.

  1. I was trying to just run it with "python" and didn't realize I had multiple python versions installed and this script is only syntactically valid for python 3.x.
  2. I needed to import the trusted keys
  3. The script was hanging because it was triggering my yubikey for signature verification
Show resolved Hide resolved contrib/verify-commits/README.md Outdated
Show resolved Hide resolved contrib/verify-commits/README.md Outdated
@jnewbery
Copy link
Member

jnewbery left a comment

ACK a75edb990516eba298e1652050dbfd28a3dd0a30. One minor style nit inline.

Show resolved Hide resolved contrib/verify-commits/README.md Outdated
@Sjors
Copy link
Member

Sjors left a comment

Concept ACK. See above style improvements, as it's difficult to read now.

I never tried this script before. Was able to run it based on your updated instructions (I wish it had a progress indicator though).

A more thorough fix for (3) is to add --no-gpg-sign in verify-commits.py (see docs, untested).

Further suggestions:

  • make it clear that "Configuration files" are already present in contrib/verify-commits
  • the linux import keys commands works fine on macOS too
Show resolved Hide resolved contrib/verify-commits/README.md Outdated
Show resolved Hide resolved contrib/verify-commits/README.md Outdated
Show resolved Hide resolved contrib/verify-commits/README.md Outdated

@jlopp jlopp changed the title Improve documentation for running verify-commits.py script Tools: improve verify-commits.py script Dec 6, 2018

@jlopp jlopp force-pushed the jlopp:verifyCommitsDocumentation branch 2 times, most recently Dec 6, 2018

@jlopp jlopp force-pushed the jlopp:verifyCommitsDocumentation branch to 45842c3 Dec 6, 2018

@jnewbery

This comment has been minimized.

Copy link
Member

jnewbery commented Dec 6, 2018

utACK 45842c3

@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented Dec 6, 2018

utACK 45842c3

@MarcoFalke MarcoFalke added this to the 0.18.0 milestone Dec 6, 2018

@jlopp

This comment has been minimized.

Copy link
Contributor Author

jlopp commented Dec 6, 2018

I tested @Sjors's "--no-gpg-sign" suggestion and it resolved my smartcard issue with running the verification script. I also tested @dongcarl's simplified key import command.

@Sjors

This comment has been minimized.

Copy link
Member

Sjors commented Dec 6, 2018

tACK 45842c3

@practicalswift

This comment has been minimized.

Copy link
Member

practicalswift commented Dec 6, 2018

utACK 45842c3

@MeshCollider

This comment has been minimized.

Copy link
Member

MeshCollider commented Dec 10, 2018

utACK 45842c3

@MeshCollider MeshCollider merged commit 45842c3 into bitcoin:master Dec 10, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MeshCollider added a commit that referenced this pull request Dec 10, 2018

Merge #14809: Tools: improve verify-commits.py script
45842c3 Improve documentation for running verify-commits.py script (Jameson Lopp)

Pull request description:

  I ran into 3 different issues while trying to run the verify-commits script for the first time and I think documenting them would help save time for future developers.

  1. I was trying to just run it with "python" and didn't realize I had multiple python versions installed and this script is only syntactically valid for python 3.x.
  2. I needed to import the trusted keys
  3. The script was hanging because it was triggering my yubikey for signature verification

Tree-SHA512: dfc7a62972ca3de528fae3c9d420c7d2d6658767a555ebbf5f4a27c04748c35ccf8bf63bfc9f264358346de0db49bfbaf2d1540793a609d81c2d9b622ee8182c

@jlopp jlopp deleted the jlopp:verifyCommitsDocumentation branch Dec 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment