-
Notifications
You must be signed in to change notification settings - Fork 87
feat: support custom writer and storage for apply and preview #62
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
feat: support custom writer and storage for apply and preview #62
Conversation
Pull Request Test Coverage Report for Build 2507038325
💛 - Coveralls |
887d1df
to
8d7c0fd
Compare
8d7c0fd
to
e743ad8
Compare
6d9b151
to
802c993
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look good to me
The interface design of the engine and the options’ pub API is not very reasonable. As here, the runtime interface shouldn't be exposed. @elliotxx and @SparkYuan, pls pay more attention to design and work out a more reasonable API interfaces of option and engine layer. And, we need to reconsider the position and functionality of the option layer, cause each individual layer has its design and cost. BTW. In general, the interface parameters include three parts, the configurations(str, int, bool..), the feature flags(bools..) and the functional structures(IO, client, etc). The functional structures are injected into the lower layer by the middle layer, and obtained by the factory provided by the lower layer. We can design and implement APIs more standardly. |
@ldxdl OK, the interface design will continue to be optimized, and we will submit PR separately to solve. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Purpose:
github.com/pterm/pterm
package to the latest version