internal: Consolidate duplicate Ticker classes and document package structure#327
internal: Consolidate duplicate Ticker classes and document package structure#327
Conversation
…tructure - Consolidate wvlet.uni.control.Ticker into wvlet.uni.rx.Ticker - Add read() and tick() aliases to rx.Ticker for API compatibility - Update cache module to use rx.Ticker directly - Re-export Ticker from control package for backwards compatibility - Add Package Structure section to README Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @xerial, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the project's time-keeping mechanism by consolidating a duplicate Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively consolidates the duplicate Ticker classes by moving the implementation to uni-core and using re-exports for backward compatibility. This is a good refactoring that improves code organization. The addition of aliases to maintain API compatibility with the cache module is also well-executed. The documentation update in README.md is a welcome addition, though there's a small opportunity for clarification.
| | `wvlet.uni.rx` | Reactive primitives (Rx, RxVar, Ticker) | | ||
|
|
||
| ### uni | ||
|
|
||
| | Package | Purpose | | ||
| |---------|---------| | ||
| | `wvlet.uni.cli` | Terminal styling, progress bars | | ||
| | `wvlet.uni.control` | Retry, circuit breaker, resource management | | ||
| | `wvlet.uni.control.cache` | In-memory caching with TTL and LRU eviction | | ||
| | `wvlet.uni.design` | Dependency injection | | ||
| | `wvlet.uni.http` | HTTP client/server | | ||
| | `wvlet.uni.io` | Cross-platform file system abstraction | | ||
| | `wvlet.uni.json` | JSON parsing | | ||
| | `wvlet.uni.msgpack` | MessagePack serialization | | ||
| | `wvlet.uni.rx` | Extended reactive features | |
There was a problem hiding this comment.
The wvlet.uni.rx package is listed under both uni-core and uni modules. While this reflects the source code structure where a package can span multiple modules, it might be confusing for readers trying to understand the project structure. To improve clarity, consider clarifying this relationship.
For example, you could merge these two rows into a single entry that explains both parts, or add a footnote. Another option is to slightly rename them in the documentation to make the distinction clear, like wvlet.uni.rx (core) and wvlet.uni.rx (extended).
Address review feedback: merge uni-core and uni package lists into a single alphabetically-sorted table to avoid confusion about wvlet.uni.rx appearing in both modules. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
wvlet.uni.control.Tickerintowvlet.uni.rx.Tickerto eliminate code duplicationread()andtick()aliases torx.Tickerfor API compatibility with cache modulerx.TickerdirectlyTickerfrom control package for backwards compatibilityTest plan
🤖 Generated with Claude Code