Skip to content
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

1.0.0 (for just ReArch) #24

Open
8 of 11 tasks
GregoryConrad opened this issue Jan 5, 2024 · 0 comments
Open
8 of 11 tasks

1.0.0 (for just ReArch) #24

GregoryConrad opened this issue Jan 5, 2024 · 0 comments
Labels
help wanted Extra attention is needed

Comments

@GregoryConrad
Copy link
Owner

GregoryConrad commented Jan 5, 2024

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 CapsuleKey should eagerly impl Hash, Eq, PartialEq, and Debug #31
    • Now irrelevant since CapsuleKey was rewritten as trait alias, and Capsule::key just returns an impl Hash + Eq + ...
  • Consider removing SideEffectRegister::register and instead have people call effect().build(register) for until experimental-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 and CapsuleReader::as_ref
    • I'm not sure I like either that much as-is. get is redundant/non-descriptive, and as_ref breaks Rust naming conventions.
    • Update: I removed get.get. I am keeping get.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) and get(some_capsule) (for returning a clone)
  • Should the Container/ContainerReadTxn/ContainerWriteTxn's xyz_read and xyz_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
  • Consider using coroutines for the API instead of CapsuleReader and the less-ergonomic SideEffectRegistrar
    • 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 to TraditionalCapsule to accommodate the future addition of coroutines (to prevent breaking change or ugly API), and then have a private Capsule trait that TraditionalCapsule has a blanket impl for.
  • Make Capsule (or TraditionalCapsule) take From<CapsuleHandle> as input so that capsules can be defined with just CapsuleReader or SideEffectRegistrar parameters.
    • Not possible because the From<CapsuleHandle> results in an unconstrained type parameter.
  • Maybe make Container::with_read_txn, Container::with_write_txn, ContainerReadTxn, and ContainerWriteTxn 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
@GregoryConrad GregoryConrad changed the title 1.0.0 (for ReArch) 1.0.0 (for just ReArch) Jan 5, 2024
@GregoryConrad GregoryConrad added the help wanted Extra attention is needed label Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant