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
Rename Script::empty
to Script::new
#1925
Conversation
2d60c82
to
36d072e
Compare
SGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 36d072e
The `empty` constructor is mis-named for the following reasons: - Non-uniform with `ScriptBuf::new` - Non-standard with respect to stdlib which uses `Path::new` and `PathBuf::new` (on which we based the `Scritp`/`ScriptBuf`) Rename the function to `new`, put it at the top of the impl block while we are at it.
Rebased on master (to pick up #1927). No other changes. |
36d072e
to
9787ba6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 9787ba6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 9787ba6
55be538 policy: Add refactor carve out (Tobin C. Harding) Pull request description: I have managed to burn out or bore our reviewers/maintainers. Getting two acks is becoming increasingly difficult. I've pestered everyone to the limit that I feel socially comfortable doing so, and as such, am requesting a carve out to the 2-ACK before merge rule. The primary justification is that I feel we should have a bit more of BDFL and a bit less total consensus if we are to push forwards. ### Example PRs where this change would apply - #1925 - #1854 - #1862 ACKs for top commit: elichai: I agree this makes sense for refactors ACK 55be538 apoelstra: ACK 55be538 sanket1729: ACK 55be538. Same reasons as apoelstra. And this is only for re-factors that are not adding any new features. RCasatta: ACK 55be538 Tree-SHA512: a5e206252015f49245ed282a3be7a35760d16f94dc6e60f31edf589a41ef642eba52a3bd7d1375b6033f3cf0128f47beee4f03e59cad151c64eedd71ac98baac
The
empty
constructor is mis-named for the following reasons:ScriptBuf::new
Path::new
andPathBuf::new
(on which we based theScritp
/ScriptBuf
)Rename the function to
new
, put it at the top of the impl block while we are at it.