Skip to content

Conversation

@Ilyes512
Copy link
Contributor

@Ilyes512 Ilyes512 commented Sep 28, 2025

Summary

See #46

@Ilyes512 Ilyes512 force-pushed the add-fromSchema branch 2 times, most recently from dd71106 to 2d87985 Compare September 29, 2025 18:32
@Ilyes512 Ilyes512 changed the title Added fromSchema to ValidatorBuilder Extended ValidatorBuilder so (absolute) urls are resolved as well Sep 30, 2025
Copy link
Owner

@osteel osteel left a comment

Choose a reason for hiding this comment

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

Hi, sorry for the delay. A general comment for now – I appreciate that you made an extension point for one to create their own factory, but it feels a bit like overkill to me. I'd rather only add extension points when the need truly arises, and I don't expect it to be the case here. Besides, we'd need to document it and that'd make the API more complex as a result. Finally, and this may be a more important point, the interface imposes to return instances of cebe\openapi\SpecObjectInterface, which doesn't belong to this package and is out of our control

@Ilyes512
Copy link
Contributor Author

Ilyes512 commented Oct 8, 2025

OK. I understand. I solved my issue by using the 2 underlying dependencies directly. Thanks for your time!

@Ilyes512 Ilyes512 closed this Oct 8, 2025
@osteel
Copy link
Owner

osteel commented Oct 9, 2025

Sorry – I didn't mean for you to close this! I was only saying that I don't think the extension point is necessary/desirable 🙂

Would still be nice to add support for URLs, but I would simplify your PR quite a bit, basically.

I'd understand if you'd lost interest now that you solved your issue though – if that's the case, I'll probably do this myself now that I've got a good idea of how to do it (thanks to you)

@Ilyes512
Copy link
Contributor Author

Ilyes512 commented Oct 9, 2025

Sorry – I didn't mean for you to close this! I was only saying that I don't think the extension point is necessary/desirable 🙂

Would still be nice to add support for URLs, but I would simplify your PR quite a bit, basically.

I'd understand if you'd lost interest now that you solved your issue though – if that's the case, I'll probably do this myself now that I've got a good idea of how to do it (thanks to you)

Ah, I was under the assumption that because I am returning SpecObjectInterface this would not be possible.

And the main reason I opted for a factory (interface) was to make it (unit) testable.

And I also agreed with the fact that it's feels more complicated than necessary.

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.

2 participants