transaction: Rename is_coin_base to is_coinbase #1796
Conversation
|
Lolll looks like we need to stick |
b2e90ea to
5ad3da2
Compare
| @@ -981,10 +981,19 @@ impl Transaction { | |||
| /// transaction. It is impossible to check if the transaction is first in the block, so this | |||
| /// function checks the structure of the transaction instead - the previous output must be | |||
| /// all-zeros (creates satoshis "out of thin air"). | |||
| pub fn is_coin_base(&self) -> bool { | |||
| #[doc(alias = "is_coin_base")] // method previously had this name | |||
There was a problem hiding this comment.
Interesting, is the idea to remove this when we remove the deprecated function? I've never used this attribute myself.
There was a problem hiding this comment.
I'd keep it for as long as we can tolerate it. Anyone might remember the old name and search for it. It doesn't hurt I guess, it only makes it show up in search when you search for that name.
There was a problem hiding this comment.
Yeah, I wouldn't mind keeping it forever. Not just because it had this name but also that it's a name someone might think of.
|
Looks like "NEXT_RELEASE" will need to go in in the "note = NEXT RELEASE" field. We should grep for "NEXT.RELEASE" too so we don't miss "NEXT-RELEASE" or "NEXT RELEASE", and leave that in released code which would be pretty embarrassing. |
|
We could also allow the lint in non-release PRs and enable it in release PRs. |
I like this. I think we do it by |
|
Grrrr at clippy enforcement, it's so unproductive! So I will try to silence clippy for non-release CI then? Is that the consensus? Let me see if I can figure out how to do that. |
Keep the old method as deprecated and add doc alias. Also change internal usage of the method.
5ad3da2 to
dad3abd
Compare
|
Hmm, I'm not sure how the CI is built, but running clippy in the release job would require having nightly cargo in that job and I don't think we want that? I went for what Tobin suggested instead, renaming to |
|
Why would nightly be a problem? Sure there's a risk that release gets stalled a bit due to a nightly bug but worst case we could turn it off. |
Nightly is generally a problem, right? What if a regression in nightly actually makes the release go through while it shouldn't? In fact I'd say it makes most sense to test the release in MSRV. Idk exactly what the release command checks for
8000
and how it differs from Anyway, I think like it's done in my latest commit should make sense. |
|
I believe we should test on all versions. |
|
CI fail is a formatting issue in patch 1 bro. Your favourite :) |
|
pushed a fix |
|
Lol, just noticed the author of the last commit. |
Alternative to #1795.
Keep the old method as deprecated and add doc alias. Also change internal usage of the method.