juergbi commented on PR #2119: URL: https://github.com/apache/buildstream/pull/2119#issuecomment-4517677498
@abderrahim Do you have any functional concerns or is your only concern that the callback might not be the the optimal implementation choice? As the callback mechanism is an implementation detail, not part of the public API (and also not a lot of code), I think we should get this merged as is, if there are no functional concerns. We can always tweak the implementation later if we can come up with a better approach. If you think the following alternative I suggested in an earlier comment would be significantly better, I'm happy to give this a try. But could also be done as potential follow-up instead of blocking this bug fix. > Another option could be to hand over the provenance nodes to the `Project` object and move the check to the `Project` class, invoked at the end of the second loading pass. Should functionally be equivalent, just implemented without callback. Not sure whether it's really better, though. -- 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]
