Move and rename crate::policy::semantic::Policy to crate::r#abstract::Abstract
#604
Conversation
We can import types to test in unit tests using `use super::*`.
|
There will be some grammar mistakes in docs 'a' vs 'an' because of semantic to abstract change. Will fix later. |
tcharding
force-pushed
the
09-28-mv-policy
branch
from
16dc092
to
1b99d1d
Compare
We used "semantic" so as not to clash with the reserved Rust keyword "abstract", however there is a language feature to deal with such name clashes - the raw identifier syntax `r#foo`. Use `r#abstract` and rename the `policy/semantic.rs` file to `policy/abstract.rs`. There is more re-naming to do to complete the semantic -> abstract change but here we _only_ do the module name to ease review.
The `Liftable` trait is its own thing, it defines a trait to lift miniscript, descriptors, and concrete policies to an abstract policy. Move the `Liftable` and associated error type and impls to a new `lift` module that lives under the recently created `crate::r#abstract` module. Note we alse remove the `Semantic` and `Concrete` public re-names/re-exports from `crate::policy` because we are transitioning away from using the term "semantic", however to minimise the changes in this patch we add a bunch of `use crate::policy::concrete::Policy as Concrete` statements.
We are moving away from the usage of "semantic" as a synonym for "abstract". Use `Abstract` instead of `Semantic` as the type alias for `crate::policy::r#abstract::Policy`.
Make it explicit that the `Policy` returned by `lift` in the `descriptor::tr` module is a `r#abstract::Policy`.
The abstract policy is different from the concrete policy because it is only part of rust-miniscript and not miniscript in general. To help differentiate it move it to a separate module. Note to reviewers, `git` couldn't work out that this is basically a file move so I've left the `src/policy/abstract.rs` file in the repo but it is not included in the build. Reviewers can do `diff src/policy/abstract.rs src/abstract/mod.rs` to ease review. It will be removed in the next patch.
We do not build this file in the build, remove it.
Use the identifier `Abstract` to further disambiguate the abstract policy and the concrete policy. This has the nice advantage of removing a few usages of the kind-of-ugly `r#abstract::Policy`.
tcharding
force-pushed
the
09-28-mv-policy
branch
from
1b99d1d
to
b79be87
Compare
|
TODO: Remove the serialization impl from |
|
Sorry, but I think we should keep the module name as What if we called the module Or we could keep it as |
|
No sweat, I'll have another play with it. |
|
I did #607 so we can see what its like, I will need to close the unwanted one and [re]write the PR description. |
|
I don't think this is going anywhere |
Based on discussion in #603. This requires #603 to be updated.
Done as a series of steps, I don't usually do PRs like this but after reviewing the recent
bitcoin_ioPR I think it does help for making review easier when there are a load of simple but slightly different changes involved in getting from one state to another. Can squash, merge, or re-split if required. Please comment on the patch series style if you have any suggestions on how I can make review easier.Draft for feedback please, especially the last patch - the struct rename from
PolicytoAbstract. Note also this leavesconcrete::Policynamed as is.