-
-
Notifications
You must be signed in to change notification settings - Fork 1
[WIP] Refactor capacity estimator routine #17
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
Conversation
Co-authored-by: streamich <9773803+streamich@users.noreply.github.com>
|
Copilot, please resume the task. |
Co-authored-by: streamich <9773803+streamich@users.noreply.github.com>
Task resumed and completed successfully! The capacity estimator routine has been fully refactored to use a centralized system. All codegenCapacityEstimator methods have been removed from individual type classes and replaced with centralized functions. The main router dispatches to appropriate estimators based on type kind. All tests pass, confirming functionality is preserved. (commit b0b43b8) |
streamich
left a comment
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.
Create a standalone codegen() function inside the src/codegen/capacity/ folder, which receives a type and generates a capacity estimator for it, something like:
const codegen = (
type: ...
options: Omit<CapacityEstimatorCodegenContextOptions, 'type'>,
): CompiledCapacityEstimator => {
const ctx = new CapacityEstimatorCodegenContext({
system: this.system,
...options,
type: this as any,
});
const r = ctx.codegen.options.args[0];
const value = new JsExpression(() => r);
// Use the centralized router instead of the abstract method
estimateCapacity(ctx, value, type);
return ctx.compile();
}
Use this generate() method inside abstract type, where needed.
Create index.ts file in capacity codegen module and export generate() and other useful routines from it.
Add any missing tests to capacity generator.
src/codegen/capacity/estimators.ts
Outdated
|
|
||
| type EstimatorFunction = (ctx: CapacityEstimatorCodegenContext, value: JsExpression, type: Type) => void; | ||
|
|
||
| function normalizeAccessor(key: string): string { |
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.
Use ArrowFunctions throughout this file for standalone functions.
| function normalizeAccessor(key: string): string { | |
| const normalizeAccessor = (key: string): string => { |
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.
Updated all standalone functions in estimators.ts to use arrow function syntax. (commit 232218c)
src/codegen/capacity/estimators.ts
Outdated
| ); | ||
| } | ||
|
|
||
| export function estimateBooleanCapacity(ctx: CapacityEstimatorCodegenContext, value: JsExpression): void { |
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.
For functions exportable from this module use short names, such as "bool" instead of "estimateBooleanCapacity":
| export function estimateBooleanCapacity(ctx: CapacityEstimatorCodegenContext, value: JsExpression): void { | |
| export const bool = (ctx: CapacityEstimatorCodegenContext, value: JsExpression): void => { |
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.
Renamed all estimator functions to use short names: "bool", "num", "str", "bin", "const_", "arr", "tup", "obj", "map", "ref", "or", "any" instead of the longer "estimateXxxCapacity" names. (commit 232218c)
src/codegen/capacity/router.ts
Outdated
| * Main router function that dispatches capacity estimation to the appropriate | ||
| * estimator function based on the type's kind. | ||
| */ | ||
| export function estimateCapacity(ctx: CapacityEstimatorCodegenContext, value: JsExpression, type: Type): void { |
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.
Rename this function to generate() and move it to estimators.ts file.
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.
Renamed the function to generate() and moved it from router.ts to estimators.ts. The router.ts file has been completely removed since all functionality is now centralized in estimators.ts. (commit 232218c)
Co-authored-by: streamich <9773803+streamich@users.noreply.github.com>
... Created the standalone |
Co-authored-by: streamich <9773803+streamich@users.noreply.github.com>
streamich
left a comment
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.
Run yarn format:fix to reformat codebase.
Run yarn lint and yarn lint:fix to fix any linter issues.
src/codegen/capacity/index.ts
Outdated
| export { | ||
| generate, | ||
| codegen, | ||
| any, | ||
| bool, | ||
| num, | ||
| str, | ||
| bin, | ||
| const_, | ||
| arr, | ||
| tup, | ||
| obj, | ||
| map, | ||
| ref, | ||
| or, | ||
| } from './estimators'; |
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.
Use star export.
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.
Updated index.ts to use star export (export * from './estimators') instead of explicit named exports. (commit 232218c)
Co-authored-by: streamich <9773803+streamich@users.noreply.github.com>
Ran |
streamich
left a comment
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.
LGMT
|
🎉 This PR is included in version 1.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fixes #9.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.