Skip to content
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

setup: On Bionic, the kcov dependency is now opt-in #13462

Merged

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented Jun 1, 2020

This resolves the immediate problem of drake-apt.csail.mit.edu certificate "expiry" (for buggy clients).

However, it is also a useful improvement towards #13168 optional prerequisites. While the mechanism of this may change (--with-kcov spelling), the idea that extra developer tools should be opt-in is one that should remain.

Relates #13475.


This change is Reviewable

@jwnimmer-tri

This comment has been minimized.

@jwnimmer-tri

This comment has been minimized.

@jwnimmer-tri jwnimmer-tri marked this pull request as ready for review June 1, 2020 13:49
@jwnimmer-tri
Copy link
Collaborator Author

jwnimmer-tri commented Jun 1, 2020

@jwnimmer-tri
Copy link
Collaborator Author

+@jamiesnape for feature review, please.

Copy link
Contributor

@jamiesnape jamiesnape left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 3 of 3 files at r1.
Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers

@jwnimmer-tri
Copy link
Collaborator Author

+@SeanCurtis-TRI for platform review per schedule, please.

Copy link
Contributor

@SeanCurtis-TRI SeanCurtis-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:LGTM: With one punctuation comment.

Reviewed 3 of 3 files at r1, 1 of 1 files at r2.
Reviewable status: 1 unresolved discussion, labeled "do not merge" (waiting on @jwnimmer-tri)


setup/ubuntu/source_distribution/install_prereqs.sh, line 27 at r2 (raw file):

# On Bionic, developers must opt-in to kcov support; it comes in with the
# non-standard package name kcov-35 via a Drake-specific PPA

nit: punctuation at the end of the sentence.

Copy link
Contributor

@SeanCurtis-TRI SeanCurtis-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r3.
Reviewable status: 1 unresolved discussion, labeled "do not merge" (waiting on @jwnimmer-tri)

@jwnimmer-tri jwnimmer-tri merged commit 7920083 into RobotLocomotion:master Jun 1, 2020
@jwnimmer-tri jwnimmer-tri deleted the bionic-kcov-optional branch June 1, 2020 15:07
@jwnimmer-tri
Copy link
Collaborator Author

Skipping most rebuilds after a bash-comment-only push.

@jamiesnape jamiesnape removed their assignment Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants