1.0.0 (for just ReArch)
GregoryConrad opened this issue · 0 comments
GregoryConrad commented
This issue only covers the 1.0.0 for the rearch
crate. The other crates are not close to a 1.0, mostly due to limitations in the Rust compiler.
TODO:
- Get a few code reviews of the library
- Garner more feedback, ideally from several users adopting the library
- Ideally some comments on my decisions in #31
- Now irrelevant since CapsuleKey was rewritten as trait alias, and
Capsule::key
just returns animpl Hash + Eq + ...
- Now irrelevant since CapsuleKey was rewritten as trait alias, and
- Consider removing
SideEffectRegister::register
and instead have people calleffect().build(register)
for untilexperimental-api
stabilizes- Yes, this does suck a little, but it also ensures there won't be any redundancy once it does stabilize
- Update: I'm currently inclined to just keep it and deprecate it when no longer needed
- Decide on the naming for
CapsuleReader::get
andCapsuleReader::as_ref
- I'm not sure I like either that much as-is.
get
is redundant/non-descriptive, andas_ref
breaks Rust naming conventions. - Update: I removed
get.get
. I am keepingget.as_ref
as-is for now, but am looking for new naming suggestions. - Note that the final api ideally will be like
get.as_ref(some_capsule)
andget(some_capsule)
(for returning a clone)
- I'm not sure I like either that much as-is.
- Should the
Container
/ContainerReadTxn
/ContainerWriteTxn
'sxyz_read
andxyz_read_ref
be named as such?- Again, maybe we should encourage the low-cost options by default so we can delete the clone variants and then (maybe) just
xyz_read_ref
->xyz_read
? - Update: non-issue, since all methods were moved under the
experimental-txn
feature
- Again, maybe we should encourage the low-cost options by default so we can delete the clone variants and then (maybe) just
- Consider using coroutines for the API instead of
CapsuleReader
and the less-ergonomicSideEffectRegistrar
- The issue is that this may be a 2.0.0 feature since who knows how far away coroutines are
- I worry about how this will work with an LSP. Such a proc-macro really defies what the analyzer is expecting.
- This would enable a syntax resembling (ideally without a macro, but idk if that is feasible):
#[capsule]
fn count_manager() -> (u32, impl CData + Fn()) {
let (state, set_state) = yield effects::State::<Cloned<_>>(1234);
(state, move || set_state(state + 1))
}
#[capsule]
fn count_capsule() -> u32 {
yield Get(count_manager).0
}
- Maybe rename
Capsule
toTraditionalCapsule
to accommodate the future addition of coroutines (to prevent breaking change or ugly API), and then have a privateCapsule
trait thatTraditionalCapsule
has a blanket impl for. - Make
Capsule
(orTraditionalCapsule
) takeFrom<CapsuleHandle>
as input so that capsules can be defined with justCapsuleReader
orSideEffectRegistrar
parameters.- Not possible because the
From<CapsuleHandle>
results in an unconstrained type parameter.
- Not possible because the
- Maybe make
Container::with_read_txn
,Container::with_write_txn
,ContainerReadTxn
, andContainerWriteTxn
crate private for a 1.0.0 to allow for extensibility or changes in the future- Although I do understand the use-cases for making them public for others to interop with.
- Maybe we can make an "experimental" feature so people that want to use them probably can, at the cost of having to pin ReArch to a particular version.
- Update: moved all of these APIs under the
experimental-txn
feature.
- Test
MockCapsuleReaderBuilder