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

Revert encode_to_fmt API change #77

Merged
merged 2 commits into from
Aug 12, 2022

Conversation

jkczyz
Copy link
Contributor

@jkczyz jkczyz commented Aug 12, 2022

Maintaining the original error type allows for making a minor release since it won't break the API. This lets indirect consumers of the crate through rust-bitcoin's module re-export to use new functions (e.g., encode_without_checksum) without a new rust-bitcoin release.

@jkczyz
Copy link
Contributor Author

jkczyz commented Aug 12, 2022

Should this be merged into master and later reverted? Or into a separate release branch?

@apoelstra
Copy link
Member

I think we should just merge into master and later revert it. No need for extra process for such a low-activity crate.

@apoelstra
Copy link
Member

Can you rebase now that #76 is merged?

Maintaining the original error type allows for making a minor release
since it won't break the API. This lets indirect consumers of the crate
through rust-bitcoin's module re-export to use new functions (e.g.,
encode_without_checksum) without a new rust-bitcoin release.
@jkczyz jkczyz force-pushed the 2022-08-revert-error-api branch from 8e7fdce to 3693935 Compare August 12, 2022 16:45
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 3693935

@apoelstra
Copy link
Member

If you add a commit which bumps the minor version number I can publish a release right after merging.

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 0d01fad

@apoelstra apoelstra merged commit d12513f into rust-bitcoin:master Aug 12, 2022
@apoelstra
Copy link
Member

Thanks! Tagged, signed and published.

@jkczyz
Copy link
Contributor Author

jkczyz commented Aug 13, 2022

Thanks! Tagged, signed and published.

Thanks for the quick turnaround on this! I had to add an explicit dependency in our crate otherwise I'd still get 0.9.0 from rust-bitcoin even after a cargo clean. But no conflicting definitions this time.

@apoelstra
Copy link
Member

Did you try cargo update?

@jkczyz
Copy link
Contributor Author

jkczyz commented Aug 15, 2022

Ah, right. 😅 Forgot the Cargo.lock file dictates which version is used. Works without the explicit dependency.

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.

2 participants