8000 Remove accidentally-exported internal macros by Kixunil · Pull Request #1177 · rust-bitcoin/rust-bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove accidentally-exported internal macros #1177

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
Merged
merged 1 commit into from
Aug 11, 2022
Merged

Remove accidentally-exported internal macros #1177

merged 1 commit into from
Aug 11, 2022

Conversation

Kixunil
Copy link
Collaborator
@Kixunil Kixunil commented Aug 9, 2022

Closes #1176

@Kixunil Kixunil added bug P-high High priority labels Aug 9, 2022
@Kixunil Kixunil marked this pull request as ready for review August 9, 2022 12:41
@Kixunil Kixunil requested review from tcharding and apoelstra August 9, 2022 12:42
@Kixunil Kixunil mentioned this pull request Aug 9, 2022
Merged
apoelstra
apoelstra previously approved these changes Aug 9, 2022
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 c9f2aae0c35a684e51f9273333c25f1b13d1d4ee

tcharding
tcharding previously approved these changes Aug 9, 2022
View reviewed changes
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 c9f2aae0c35a684e51f9273333c25f1b13d1d4ee

sanket1729
sanket1729 previously approved these changes Aug 11, 2022
View reviewed changes
Copy link
Member
@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

ACK c9f2aae0c35a684e51f9273333c25f1b13d1d4ee

@sanket1729
Copy link
Member

I think rebase would fix this after we merged #1175 . But I don't mind merging it as this should pass on merge commit

@Kixunil
Copy link
Collaborator Author
Kixunil commented Aug 11, 2022

Hmm, I thought GitHub would test against current master, not against one that was there at the time of opening.

@Kixunil Kixunil dismissed stale reviews from sanket1729, tcharding, and apoelstra via 94eeabf August 11, 2022 09:11
@Kixunil Kixunil force-pushed the unexport-internal-macros branch from c9f2aae to 94eeabf Compare August 11, 2022 09:11
@sanket1729
Copy link
Member

Hmm, I thought GitHub would test against current master, not against one that was there at the time of opening.

No it does not. One of the main reasons that we have the merge script that runs tests on the merge commit. It is also possible to PR with green lights passes checks from its own CI config, but fails master.

sanket1729
sanket1729 approved these changes Aug 11, 2022
View reviewed changes
Copy link
Member
@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

reACK 94eeabf

apoelstra
apoelstra approved these changes Aug 11, 2022
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 94eeabf

@apoelstra apoelstra merged commit 3c758c7 into rust-bitcoin:master Aug 11, 2022
@Kixunil Kixunil deleted the unexport-internal-macros branch August 11, 2022 14:01
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

@sanket1729 sanket1729 sanket1729 approved these changes

@tcharding tcharding tcharding left review comments

Assignees
No one assigned
Labels
bug P-high High priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Messed up release
4 participants
@Kixunil @sanket1729 @apoelstra @tcharding
0