-
Notifications
You must be signed in to change notification settings - Fork 17
Fix: allow multiple dependencies #298
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
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //src:license-checkStatus: Click to expand output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a bug where only the last external dependency was being retained when multiple dependencies were configured. The fix moves the initialization of config.needs_external_needs = [] from inside the add_external_needs_json function (which is called in a loop) to the connect_external_needs function (before the loop starts).
- Relocated list initialization from
add_external_needs_jsontoconnect_external_needs - Ensured that multiple external needs can be accumulated correctly instead of being overwritten
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The created documentation from the pull request is available at: docu-html |
ubmarco
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, thanks for the fix.
Every addition of an external dependency has reset the list. Instead reset the list once, before adding dependencies.