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

Update github macos setup for drake prereqs #297

Merged

Conversation

nicolecheetham
Copy link
Collaborator

@nicolecheetham nicolecheetham commented Aug 27, 2024

Simplifying DEE approach of installing drake prereqs for macOS

Issue: #61


This change is Reviewable

@nicolecheetham nicolecheetham marked this pull request as draft August 28, 2024 13:18
@nicolecheetham nicolecheetham marked this pull request as ready for review September 9, 2024 20:10
Copy link
Collaborator Author

@nicolecheetham nicolecheetham left a comment

Choose a reason for hiding this comment

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

+@BetsyMcPhail for feature review

Reviewable status: all discussions resolved, LGTM missing from assignee betsymcphail, platform LGTM missing (waiting on @BetsyMcPhail)

Copy link
Contributor

@BetsyMcPhail BetsyMcPhail left a comment

Choose a reason for hiding this comment

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

:lgtm: assuming that CI passes

Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: :shipit: complete! all discussions resolved, platform LGTM from [betsymcphail] (waiting on @nicolecheetham)

Copy link
Collaborator Author

@nicolecheetham nicolecheetham left a comment

Choose a reason for hiding this comment

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

+@xuchenhan-tri for platform review

Reviewable status: all discussions resolved, LGTM missing from assignee xuchenhan-tri, platform LGTM from [betsymcphail] (waiting on @xuchenhan-tri)

Copy link

@xuchenhan-tri xuchenhan-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 5 of 5 files at r2, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee xuchenhan-tri, platform LGTM from [betsymcphail] (waiting on @nicolecheetham)


scripts/setup/mac/install_prereqs line 7 at r2 (raw file):


# Installs a /usr/local/bin/bazel symlink that conflicts with that installed by
# the bazel formula in Homebrew.

BTW, the comment here seems to be incompatible with the code below. Am I just not getting it?

Code quote:

# Installs a /usr/local/bin/bazel symlink that conflicts with that installed by
# the bazel formula in Homebrew.

Copy link

@xuchenhan-tri xuchenhan-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:

Reviewable status: 1 unresolved discussion, platform LGTM from [betsymcphail, xuchenhan-tri] (waiting on @nicolecheetham)

Copy link
Collaborator Author

@nicolecheetham nicolecheetham left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 unresolved discussion, platform LGTM from [betsymcphail, xuchenhan-tri] (waiting on @BetsyMcPhail and @xuchenhan-tri)


scripts/setup/mac/install_prereqs line 7 at r2 (raw file):

Previously, xuchenhan-tri wrote…

BTW, the comment here seems to be incompatible with the code below. Am I just not getting it?

You are correct. The comment is supposed to start with "Uninstalls" as to properly describe the code below. I have updated so this is the case.

Copy link

@xuchenhan-tri xuchenhan-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, all commit messages.
Reviewable status: :shipit: complete! all discussions resolved, platform LGTM from [betsymcphail, xuchenhan-tri] (waiting on @nicolecheetham)

Copy link
Contributor

@BetsyMcPhail BetsyMcPhail 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, all commit messages.
Reviewable status: :shipit: complete! all discussions resolved, platform LGTM from [betsymcphail, xuchenhan-tri] (waiting on @nicolecheetham)

Copy link
Contributor

@BetsyMcPhail BetsyMcPhail left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all discussions resolved, platform LGTM from [betsymcphail, xuchenhan-tri] (waiting on @nicolecheetham)

Copy link
Contributor

@BetsyMcPhail BetsyMcPhail left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: :shipit: complete! all discussions resolved, platform LGTM from [betsymcphail, xuchenhan-tri] (waiting on @nicolecheetham)

@BetsyMcPhail BetsyMcPhail merged commit 7ffcfdf into RobotLocomotion:main Sep 11, 2024
6 checks passed
@nicolecheetham nicolecheetham deleted the dde_macsonoma_refactor branch September 11, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants