-
Notifications
You must be signed in to change notification settings - Fork 59
feat(go-feature-flag)!: Introduce in-process evaluation + tracking #1365
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
7cd00c1
to
5e76cfd
Compare
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
8bdbbc8
to
bfc964c
Compare
Signed-off-by: Thomas Poignant <[email protected]>
bfc964c
to
3cc2b65
Compare
Signed-off-by: Thomas Poignant <[email protected]>
Hey @thomaspoignant, I can configure Gemini here if you'd like. We've only enabled it in a few repos but it has worked well so far! |
Yes I will be awesome if you can enable it here. |
/gemini review |
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.
Code Review
This pull request is a complete and impressive rebuild of the GO Feature Flag provider, introducing in-process evaluation via WebAssembly and remote evaluation via OFREP. The new architecture is well-structured, modular, and a significant improvement. My review focuses on ensuring documentation consistency and improving the maintainability of the build process.
libs/providers/go-feature-flag/src/lib/go-feature-flag-provider-options.ts
Show resolved
Hide resolved
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
ecb866f
to
c5ef0d1
Compare
Signed-off-by: Thomas Poignant <[email protected]>
/gemini review |
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.
Code Review
This pull request represents a significant and impressive overhaul of the GO Feature Flag provider, introducing in-process evaluation via WebAssembly and remote evaluation through OFREP. The new architecture is well-structured, with clear separation of concerns, and the addition of comprehensive documentation and tests is a great improvement.
My review focuses on a few minor areas for refinement:
- Ensuring consistent timestamp generation for tracking events.
- Correcting the error handling example in the README to align with the actual exceptions thrown.
- Improving the logic for batching and publishing events to avoid potential race conditions.
libs/providers/go-feature-flag/src/lib/go-feature-flag-provider.ts
Outdated
Show resolved
Hide resolved
libs/providers/go-feature-flag/src/lib/service/event-publisher.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
/gemini review |
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.
Code Review
This is an impressive and comprehensive rewrite of the GO Feature Flag provider. The introduction of in-process evaluation via WebAssembly is a significant architectural improvement, and the new modular structure is much cleaner and more maintainable. The code quality is high, and the test coverage appears to be thorough. My feedback focuses on a few areas to improve robustness, particularly around asynchronous operations and potential race conditions.
libs/providers/go-feature-flag/src/lib/evaluator/inprocess-evaluator.ts
Outdated
Show resolved
Hide resolved
libs/providers/go-feature-flag/src/lib/service/event-publisher.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
1091412
to
351ba45
Compare
Signed-off-by: Thomas Poignant <[email protected]>
351ba45
to
9b990d9
Compare
/gemini review |
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.
Code Review
This pull request is a significant and impressive rewrite of the GO Feature Flag provider, introducing in-process evaluation using WebAssembly and support for remote evaluation via OFREP. The changes are well-structured, with clear separation of concerns into evaluators, services, and hooks. The addition of a build script to manage the WASM binary is a thoughtful touch. My feedback focuses on improving the developer experience by clarifying the documentation and enhancing the flexibility of the build scripts. Overall, this is a high-quality contribution that modernizes the provider.
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
This PR
This is a complete rebuild of the GO Feature Flag provider.
This version is using a webassembly module to do in-process evaluation inside the provider.
You can still use the remote evaluation if you prefer through the OFREP endpoint of GO Feature Flag.
Things to know: