malliaridis commented on PR #2605: URL: https://github.com/apache/solr/pull/2605#issuecomment-2274591704
> 2. The biggest downside in the code for me is how boilerplate heavy it is. There are Component interfaces, which have Component implementations, which then reference Models, which come from Stores, which fetch the data via API calls. Those abstractions all make sense individually. But they're the main reason probably that this turned into an 8k line PR! You are right. There are multiple layers that abstract API, logic and UI from each other, resulting in a lot of boilerplate code. The repetitiveness is also a side-effect of applying the same patterns to all componets. Besides the drawback of the quantity in changes, there are two main benefits that may be achieved with that approach: testing components / individual parts becomes much easier, and new developers have an easier start because they can follow the structure and patterns from other components when they are working on a new component. I wanted to write some tests for completion, but I had to postpone it for the time being. > Positing community consensus on taking this forward, I think we'll need to break this into smaller pieces that are less intimidating to folks. I suspect you've already thought a bit about where to draw some of those lines, but I have at least some hunches we could talk over at some point. (Split "Java Properties" and "logging" pages into different PRs, slim the theme down to only support a single mode, commit the theme separately, etc.) You are completely right on that. It is possible, and this was the plan once we work on the Jira part of the migration, to split the components into separate PRs and tickets. This would allow in the long run an easier integration of new components. Contributors will also have to worry only about a single component and not the entire application when working on a ticket, PR, feature or bugfix. The POC includes already two of the components for demonstration, so that it covers almost all parts of the Compose app (including navigation) that are crucial to get started. The only thing I can recall at the moment and that is missing is probably the explicit error handling, but that is something I have already worked on in other projects and shouldn't be a problem integrating. > I've left a few comments inline. Thank you very much for your time reviewing this. As you noticed, this POC still needs some work and cleanup before moving forward. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
