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

Add support for macOS Big Sur #208

Closed
wants to merge 6 commits into from
Closed

Conversation

rotu
Copy link
Contributor

@rotu rotu commented Sep 9, 2020

No description provided.

@cottsay
Copy link
Member

cottsay commented Sep 9, 2020

Two thoughts:
1. None of the hundreds of osx rosdep keys are going to work on a platform that detects the OS as macos, regardless of whether the same OSDetector class is used. You should use an alias, as is done for derivatives of Ubuntu and RHEL.
2. I'm not thrilled to see a mixture of refactor changes mixed in with the change you're proposing. If you believe moving stuff into that class provides value, I think you should open a separate PR, and we'll discuss whether it is worth touching the lines and complicating a git blame process.

@cottsay
Copy link
Member

cottsay commented Sep 9, 2020

None of the hundreds of osx rosdep keys are going to work on a platform that detects the OS as macos, regardless of whether the same OSDetector class is used. You should use an alias, as is done for derivatives of Ubuntu and RHEL

Nix that, the aliasing happens in rosdep, not here. Mea culpa.

@rotu
Copy link
Contributor Author

rotu commented Sep 9, 2020

Everything past Sierra is macOS, and Big Sur is not even macOS 10. Feel free to propose a better fix or only pull some of these changes.

Nevermind I'll submit issues instead

@rotu rotu closed this Sep 9, 2020
@rotu
Copy link
Contributor Author

rotu commented Sep 9, 2020

extracted to #209 and #210

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.

2 participants