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

fix: correct links and mentions to the v0.32.0 bitcoin #31

Merged

Conversation

storopoli
Copy link
Contributor

@storopoli storopoli commented May 4, 2024

In the Cargo.toml of the tests we are already using 0.32.0.
Hence, the doc links and mentions from 0.31.1 should be updated.

PS: I've added a minor inconsistency fix in 95f1401

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

utACK 95f1401

Didn't check that all the new docs.rs links work. Not sure how best to do that.

@storopoli
Copy link
Contributor Author

storopoli commented May 4, 2024

That's the purpose of the check-links.yml CI.
I've haven't been following this repo (changed to follow it from now on).
It's failing due to outdated secp256k1 docs links.
I'll correct those in a new commit here.

Give me until the end of the weekend.

@storopoli
Copy link
Contributor Author

Done!
@apoelstra can you please do a manual run of check-links.yml for the branch storopoli:storopoli/update-links-v0.32?

I don't have the permissions necessary.

@apoelstra
Copy link
Member

Hmm, I don't see a way to do it for branches outside of the repo. I think I should just ACK this PR and merge it and then we'll run it on master and see if we need to fix anything.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

utACK 586a71c

@apoelstra
Copy link
Member

BTW on master it looks like we have two(?) links to https://docs.rs/secp256k1/0.27.0/secp256k1/struct.Keypair.html which 404s.

@tcharding
Copy link
Member

Oh sorry man, I ran it before and didn't comment here.

@tcharding
Copy link
Member

I can't find those links to 0.27.0 locally?

@storopoli
Copy link
Contributor Author

storopoli commented May 5, 2024

BTW on master it looks like we have two(?) links to https://docs.rs/secp256k1/0.27.0/secp256k1/struct.Keypair.html which 404s.

Yes, but this is fixed in this PR, you've ran the check-links from the master branch.

I've ran the file checking link manually and here's the results (abbreviated with <ABBR>):

rust-bitcoin.github.io on  storopoli/update-links-v0.32 [?] took 6s 
❯ docker run -v ${PWD}:/tmp:ro --rm -i ghcr.io/tcort/markdown-link-check:stable /tmp/cookbook/src/tx_segwit-v0.md
(node:1) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)

FILE: /tmp/cookbook/src/tx_segwit-v0.md
  <ABBR>
  50 links checked.

rust-bitcoin.github.io on  storopoli/update-links-v0.32 [?] took 15s 
❯ docker run -v ${PWD}:/tmp:ro --rm -i ghcr.io/tcort/markdown-link-check:stable /tmp/cookbook/src/tx_taproot.md
(node:1) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)

FILE: /tmp/cookbook/src/tx_taproot.md

<ABBR>

  58 links checked.

Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK 586a71c

@tcharding tcharding merged commit bd008ad into rust-bitcoin:master May 6, 2024
1 check passed
@tcharding
Copy link
Member

Thanks for fixing my mistakes!

@tcharding
Copy link
Member

I'm not sure how we remember to not do this same mistake next time, currently either the dev or the reviewer has to remember. If I'm the dev again next time there is a reasonable chance I'll forget ...

@storopoli
Copy link
Contributor Author

I'll try to help the burden and enforce these in the future as well

@storopoli storopoli deleted the storopoli/update-links-v0.32 branch May 6, 2024 07:52
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