Skip to content

Conversation

@eddumelendez
Copy link
Contributor

It also supports ConnectionDetails for upcoming Testcontainers and Docker Compose service connection support.

@eddumelendez eddumelendez requested a review from a team as a code owner October 2, 2024 15:39
It also supports `ConnectionDetails`.
@hello-caleb
Copy link

Thank you for this contribution! Our apologies for the delay in reviewing your PR. As maintainers, we are working hard to cover as much as we can, and the assistance of the community is always deeply appreciated!

While the code looks sound, I would also advise updating the README.md file to note the new structure. You can find this referenced in the OpenFGA Contribution Guidelines.

Others may have some questions to confirm that the build process works end-to-end locally with the new subproject structure, given the upcoming Testcontainers and Docker Compose service connection support. But my main comment is the request for a PR for documentation updates and linking that PR to this one in this PR's description.

@rhamzeh rhamzeh changed the title Move src folder to subproject chore: move src folder to subproject Dec 4, 2024
@rhamzeh rhamzeh requested a review from jimmyjames December 4, 2024 18:42
@jimmyjames
Copy link
Collaborator

Thanks @eddumelendez! Also apologies for the extended delay here 😞.

I love this change, and its a structure I had originally envisioned, but I wonder if it will run into the same issue with nexus publishing that we encountered before? We originally envisioned a structure similar to what you have done here, but had issues with nexus publishing that didn't appear to have a nice solution (though it has been some time since then and I see the issue has many comments).

@piotrooo
Copy link

This PR LGTM ❤️

I'm just curious if naming the project spring-boot-starter is the best choice, considering it will include Testcontainers and Docker modules. Just sharing my thoughts 🤔, not saying it's wrong.

Maybe just spring-boot would be enough.

@rhamzeh
Copy link
Member

rhamzeh commented Dec 16, 2024

@eddumelendez - thanks for this PR! We're not ignoring it, we'd just like to test it with the release process taking into account what @jimmyjames has mentioned above. Do expect this to take some time as we have a few other priorities, but we'll be investigating once we're back from the holidays, so expect an approval or a clearer feedback in the first few weeks of 2025

@piotrooo piotrooo mentioned this pull request May 14, 2025
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.

5 participants