tkobayas commented on PR #5794: URL: https://github.com/apache/incubator-kie-drools/pull/5794#issuecomment-2011136567
> if > > > startCharacter and endCharacter are Descr's properties > > why they are not already populated at that specific point, and the proposed modification has to populate them ? `startCharacter` and `endCharacter` should be populated when the RuleDescr is created, and the PR fix does it --- in the same method `DRLVisitorImpl.visitRuledef`. Talking about the root cause "why we had this bug", I think of - We can improve the way to create Descr objects not to miss populating the properties in `DRLVisitorImpl` - Add more test cases In this regard, I filed https://github.com/apache/incubator-kie-drools/issues/5798 > but IIUC the problem is somewhere else (please correct me if I'm wrong, I'm simply inferring from your own comment) My comment: > The root cause was that RuleDescr.getStartCharacter() returned -1 (missed in DRLVisitorImpl), so the kbase was not updated (the error was logged). So fixed. meant that the `RuleDescr.startCharacter` was `-1` at the point. It had to be populated by `DRLVisitorImpl` beforehand. So I wanted to explain that the root cause was `DRLVisitorImpl`, not `ChangeSetBuilder.diffDescrs`. > I can't wait to talk directly with you, to get to a better understanding. Sure, we can talk on meet or zoom. -- 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]
