Skip to content

build(flagd): OS agnostic flagd provider build #998

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

Merged
merged 5 commits into from
Oct 3, 2024

Conversation

guidobrei
Copy link
Member

This PR

removes pom.xml dependency on cp command from flagd provider maven build.

Proposed changes:

  • Compile GRPC proto files directly from schemas repository
  • Use copy resources maven plugin to copy JSON schema files
  • Improve test logic to load resource file contents

Notes

Flagd Provider uses plain cp commands in maven build. This causes problems on Windows systems.

How to test

./mvnw verify will build the project (also on windows)

@guidobrei guidobrei changed the title OS agnostic flagd provider build build(flagd): OS agnostic flagd provider build Oct 3, 2024
@guidobrei guidobrei force-pushed the build/flagd-os-agnostic branch from 9c25aa0 to a7f0fba Compare October 3, 2024 09:13
Signed-off-by: Todd Baert <[email protected]>
@@ -1,184 +0,0 @@
{
"$id": "https://flagd.dev/schema/v0/flags.json",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a small change to some test files that use this schema; they now use the pulled/gitignored one.

The test files using this schema only use it for editor support (so editing the schemas locally has editor support) not in actual unit tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, there they have been used. I was wondering why there have been JSON schemas checked in in resources when they're copied to a different resource directory on build.

@toddbaert toddbaert self-requested a review October 3, 2024 16:07
@toddbaert
Copy link
Member

This is a helpful additional and makes the POM simpler too. Thanks!

@toddbaert toddbaert merged commit 27543c9 into open-feature:main Oct 3, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants