Skip to content

Comments

README Update: set completion of response verifaciton for read and derive commands#26

Merged
notmandatory merged 1 commit intobitcoindevkit:masterfrom
upjohnc:readme-update-supporte-features
Apr 21, 2025
Merged

README Update: set completion of response verifaciton for read and derive commands#26
notmandatory merged 1 commit intobitcoindevkit:masterfrom
upjohnc:readme-update-supporte-features

Conversation

@upjohnc
Copy link
Contributor

@upjohnc upjohnc commented Mar 17, 2025

Description

This is an update to the README to show that the response
verification has been added to the code for the read and derive
commands.

Notes to the reviewers

The verification for the two commands seem to be done in these
two code references

Read Trait:

https://github.com/notmandatory/rust-cktap/blob/d8258229ff080ce5adaeff65898bcc3846faa6d1/lib/src/commands.rs#L109

Derive:

@upjohnc upjohnc marked this pull request as ready for review March 17, 2025 19:33
@upjohnc
Copy link
Contributor Author

upjohnc commented Mar 18, 2025

@notmandatory @reez
I created this pull request and wanted to add reviewers. I can't seem to find how to edit the list of reviewers, so I am adding this comment to let you know this PR is up for your review.

@reez
Copy link
Collaborator

reez commented Mar 18, 2025

@notmandatory @reez I created this pull request and wanted to add reviewers. I can't seem to find how to edit the list of reviewers, so I am adding this comment to let you know this PR is up for your review.

I can't seems to edit the list of reviewers either so thanks for the ping to take a look at this PR!

@reez
Copy link
Collaborator

reez commented Mar 18, 2025

UTACK 64d16fd

@notmandatory
Copy link
Member

@upjohnc please rebase on master to fix the CI issue and then I'll get this one merged.

@upjohnc upjohnc changed the base branch from master to ci/bump_msrv_181 April 21, 2025 23:07
@upjohnc upjohnc changed the base branch from ci/bump_msrv_181 to master April 21, 2025 23:07
@notmandatory notmandatory force-pushed the readme-update-supporte-features branch from 4bd957a to 9acb78d Compare April 21, 2025 23:21
@notmandatory
Copy link
Member

I had to redo your rebase, somehow your commit ended up in front of all the recent commits on master. I'll show ya how I fixed next time I see you in the office.

@upjohnc
Copy link
Contributor Author

upjohnc commented Apr 21, 2025

@upjohnc please rebase on master to fix the CI issue and then I'll get this one merged.

Alright. I rebased and pushed. The good part is github actions is succeeding. The thing is that it shows all the commits that are new (added from rebase). I don't usually rebase, so need help with what I need to correct with all those commits showing in the git diff.

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK 9acb78d

@notmandatory notmandatory merged commit 9acb78d into bitcoindevkit:master Apr 21, 2025
6 checks passed
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