-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
add verbiage about overriding 3rdparty content #29913
Conversation
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.
Minor wording changes.
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.
A few more things I noticed while going through this.
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.
Overall this is a great addition to our contributing docs and it's going to help contributors and reviewers alike. I've added some suggestions and there are a few in flight from other reviewers. Otherwise it is looking good!
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.
LGTM, with clalancette and nuclearsandwich comments addressed.
The majority of the content is from: https://discourse.ros.org/t/release-package-requiring-custom-version-of-system-dependency/18200/6?u=tfoote Following up: #29614 (comment)
Co-authored-by: Chris Lalancette <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
Co-authored-by: Jacob Perron <[email protected]>
Co-authored-by: Jacob Perron <[email protected]>
Co-authored-by: Steven! Ragnarök <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
Co-authored-by: Tully Foote <[email protected]>
aa4b503
to
ac05b33
Compare
The majority of the content is from: https://discourse.ros.org/t/release-package-requiring-custom-version-of-system-dependency/18200/6?u=tfoote
Following up: #29614 (comment)