Make common generics more terse #573
Conversation
tcharding
force-pushed
the
07-25-rename-generics
branch
from
d2510bc to
1061593
Compare
|
sgtm. See my comments in the other PR, which match my current opinions. |
|
Awesome, thanks. How about |
|
Yep, I think |
|
Cool, just to make sure, would either of these be better?
|
|
I think |
|
Cool, I'm going to go with |
tcharding
force-pushed
the
07-25-rename-generics
branch
from
1061593 to
91e2fc8
Compare
|
Patch one is a bit of a punish to review but it can be verified by doing the changes yourself and running the formatter
No other manual changes in patch 1 |
tcharding
force-pushed
the
07-25-rename-generics
branch
from
91e2fc8 to
2c8ca20
Compare
|
I think in the second commit we should also re-export |
|
Will hold off on merging in case you want to add the extra re-export. |
Oooo, to be honest I did not actually notice that change this time around when I redid this work. I'll add the re-export as suggested. Furthermore, do we want to use |
tcharding
force-pushed
the
07-25-rename-generics
branch
from
2c8ca20 to
ec45d5f
Compare
|
I re-ordered the patches to put the alias stuff last. Added |
Oo, yeah, let's do that. |
In an effort to make the code more terse with no loss of clarity rename the crate's main two traits `MiniscriptKey` to `Key` and `ScriptContext` to `Context`. The diff is big but can be verified by simply doing a search-and-replace on `MiniscriptKey` -> `Key` and `ScriptContext` -> `Context`. Then run `cargo +nightly fmt`. No other manual changes.
Now we have `Key` instead of `MiniscriptKey` we can take advantage of the shorter name to inline the where clauses when appropriate.
We just renamed the `MinscriptKey` trait to `Key` and `ScriptContext` to `Context`. Add an alias for each of them to the original name. This helps backwards compatibility but also makes the library more ergonomic since the new names are so generic they could cause naming conflicts in downstream crates.
Add an alias to the `miniscript::context` module to maintain backwards comparability. Also, as for `ScriptContext` this may reduce potential naming conflicts downstream because "context" is not that unique. Note the alias is in `miniscript::context` but in `miniscript` there is a re-export of `Contetxt` but not one of the error.
|
The errors are a bit of a mess, changing to use |
tcharding
force-pushed
the
07-25-rename-generics
branch
from
ec45d5f to
24c11c0
Compare
|
Rebased, no other changes. |
|
Hey Tobin, sorry for the delayed response. I am waiting a few days to merge other PRs to see if we can #481 in first. |
|
Oh definitely! don't make them rebase that PR. Thanks for paying attention. |
|
One invasive clean up at a time is plenty, leaving this util there is an opportune moment to push it. |
This is #375 brought back to life, I don't have perms to re-open PRs. Includes all things from the discussion thread. The individual patches are marked wip and have foramatting changes separate, this is so reviewers can more easily see how the changes effect formatting. Also this PR is hella invasive, it can sit here until its a good time to merge then I can redo the whole thing and squash it into a single patch.
I will in no way take offence if this gets nack'ed for being too invasive :)