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.

Sign up for GitHub

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

Jump to bottom

Release tracking PR - v0.20.2-0.6.0 #67

Merged
merged 1 commit into from
Dec 9, 2023
Merged

Release tracking PR - v0.20.2-0.6.0 #67

merged 1 commit into from
Dec 9, 2023

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Nov 29, 2023 •
edited
Loading

In preparation for release bump the version and add a changelog entry.

@tcharding tcharding mentioned this pull request Nov 29, 2023
Closed
@tcharding tcharding changed the title Release tracking PR Release tracking PR - v0.20.2-0.6.0 Nov 30, 2023
@tcharding tcharding mentioned this pull request Dec 1, 2023
Closed
@tcharding tcharding force-pushed the 11-29-release branch 2 times, most recently from b041f3b to 245fa36 Compare December 1, 2023 01:30
This was referenced Dec 1, 2023
Closed
Closed
apoelstra
apoelstra reviewed Dec 4, 2023
View reviewed changes
src/lib.rs Outdated
/// The spending transaction in wire format is:
///
/// The (randomly choosen) Bitcoin transaction [aca326a7] spends one input, that is the first output
/// of [95da3445]. The spending transaction in wire format is:
Copy link
Member

Choose a reason for hiding this comment

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

In 2862208:

I don't really like the idea of using short hashes, since you can't look them up anywhere.

But if we are going to touch this code, we should drop the links to bc.i. They are a hostile actor. If we must link to a block explorer we can use blockstream.info. Or nothing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair, and fair. Will re-spin. Thanks

Copy link
Member Author

Choose a reason for hiding this comment

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

Remove links and used the whole hashes.

@apoelstra
Copy link
Member

This looks good and I think we could merge it as-is. But I think the ffi module should only contain C bindings, because that's idiomatically what users expect to find in a module called ffi. And these bindings should be exposed to the public.

@tcharding
Copy link
Member Author

tcharding commented Dec 5, 2023 •
edited
Loading

No worries.

EDIT: Deleted all the stuff I wrote as unnecessary for you to read after spending some more time playing with the PR.

@tcharding
Copy link
Member Author

Force pushed the following:

  • Remove the support for the old pre-segwit verify function, it adds no value
  • Put only the C definitions into the ffi module
  • Further improved the docs

@apoelstra
Copy link
Member

Could you move the version-bump commit to its own PR? It's easier for me to run my testing script.

@apoelstra
Copy link
Member

but utACK 5945973

@tcharding
Copy link
Member Author

Can do, no sweat!

@tcharding tcharding mentioned this pull request Dec 6, 2023
Merged
@tcharding tcharding marked this pull request as draft December 6, 2023 21:32
apoelstra added a commit that referenced this pull request Dec 6, 2023
@apoelstra
Merge #71: Add ffi module
76ada74 
01a48d1 Add public ffi module (Tobin C. Harding)
6782fd1 Remove travis config file (Tobin C. Harding)
a446115 Remove written by comment (Tobin C. Harding)
e8ff1a9 Shoosh C++ compiler (Tobin C. Harding)
8d21e3d Fix rustdoc link (Tobin C. Harding)
596e2ce Move extern C stuff to bottom of file (Tobin C. Harding)
7c1cb29 Remove wildcard import (Tobin C. Harding)
1d36f2c Move error code to bottom of file (Tobin C. Harding)
bed7cde Improve docs on verify function (Tobin C. Harding)

Pull request description:

  This is #67 without the final version bump patch.

  Do a bunch of cleanups then add an `ffi` module to encapsulate the FFI function declarations.

ACKs for top commit:
  apoelstra:
    ACK 01a48d1

Tree-SHA512: 5e951156498319c0559c5ba48da10810d3112f2372801d778b579e0ecf12bfe2c69d570c9837563d0de06272f4a8e76ae2d9a8afdc1f3a10733cbed96cc5e201
@tcharding
Bump version to 0.20.2-0.6.0
c3bccf7 
In preparation for release bump the version and add a changelog entry.
@tcharding tcharding marked this pull request as ready for review December 7, 2023 20:09
@tcharding
Copy link
Member Author

Can you merge and release please @apoelstra? Then I'll rebase #70, but that one will need a bit of thought to review because it involves changes to how we vendor the Core code.

apoelstra
apoelstra approved these changes Dec 9, 2023
View reviewed changes
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.

ACK c3bccf7

@apoelstra apoelstra merged commit a4c0cfb into rust-bitcoin:master Dec 9, 2023
@apoelstra
Copy link
Member

Tagged and published.

@tcharding
Copy link
Member Author

Legend!

@tcharding tcharding deleted the 11-29-release branch December 11, 2023 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apoelstra apoelstra apoelstra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tcharding @apoelstra