gitgabrio commented on PR #5751: URL: https://github.com/apache/incubator-kie-drools/pull/5751#issuecomment-1972811892
Hi @tkobayas , many thanks, that's great! The only think I noticed is that this document dive deep in the current implementation (e.g old vs new approach) which is very good ( 👍 ) but reading it, I could not find a description of the "design" itself (beside that `DRLParser processes a DRL text and produces an AST(abstract syntax tree). Then apply DRLVisitorImpl to generate PackageDescr following the visitor pattern. So the main work would be implementing DRLParser.g4 and DRLVisitorImpl` ). What I mean is that this document is very good to describe the current implementation, but does not provide a sort of mapping between the "technical needs"and the current code. Reason for raising this is that, in the future, 1. as we moved from different Antlr version, maybe we could switch to a completely different technology, and this document could help to distinguish what was the scope of different piece of code from what was the consequence 2. from general quality POV, it would help to verify if the current implementation, that is the result of long works and refactory, does exactly reflect the needs, or if it is flawed by some improper design choices that have been introduced long time ago and we are now keep building on top of. Does this make sense ? Do you think it could be usefull/feasible to also provide this kind of information ? Of course, it could be deferred to other ticket (and, of course, it is very pèossible that I misread/misundestood that document 😄 ) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
