-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Refactoring the ModelsBuilder TextWriter to use Interfaces and DI for more flexibility in how we build our models #17096
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
Splitting Builder and TextBuilder out into 4 interfaces that are registered as services so they now are injectable Bit of rewrite to pass all tests.
Hi there @kasparboelkjeldsen, thank you for this contribution! 👍 While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:
Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution. If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request. Thanks, from your friendly Umbraco GitHub bot 🤖 🙂 |
Hold off on reviewing, I'll fix the tests first :) |
Added "availableTypes" as parameter to avoid having to use both builderbase and textbuilder when generating.
Locally the PR now passes all integration and unit tests. It looks like I'm timing out in your pipeline here right now, but I'm ready for feedback. I know it's a doozy, but I also think it unlocks all kinds of cool things for developers. |
Hey @kasparboelkjeldsen, thanks for looking at this. On a personal note I'm really excited to see modelsbuilder getting some love. We'll get this reviewed and get back to you soon. |
Prerequisites
Description
This is based off this discussion 17047
The aim of this pull request is to enable further customization of the TextBuilder for Models generation, generating csharp classes by splitting up the existing implementation into multiple classes and interfaces that are registered for dependency injection.
I have done my best to document the new interfaces. Actual implementation yields indentical models to existing implementation. But by allowing it to run with dependency injection, package developers and others are now able to replace and decorate functionality closer to what they are trying to solve with the TextBuilder.
The new interfaces are:
IBuilderBase which is thought of as a dependency for basically anything related to generate models. (Previously Builder.cs)
ITextBuilder - extracted from the TextBuilder implementation, exposes 2 Generate methods.
ITextBuilderActions - first level down in complexity, exposing methods for writing header, usings, namespaces, class definition, properties etc.
ITextBuilderSubActions - nitty gritty level where you affect individual items of a class, such as attributes written on top of methods and properties, as well as actual property implementation.