modules: thrift: update doc and makefiles to build with macos #74557
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added tabs to the thrift doc so that building works for macos and is consistent with the rest of the documentation in terms of one tab per os. Updated client and server makefiles to discover include paths for boost and openssl. Signed-off-by: Chris Friedt <cfriedt@tenstorrent.com>
zephyrbot
added
manifest
manifest-thrift
DNM
zephyrbot
removed
manifest
manifest-thrift
DNM
pdgendt
reviewed
Jun 20, 2024
Comment on lines
+19
to
+21
| ifeq ($(OS),Darwin) | ||
| # get Homebrew prefix | ||
| HOMEBREW_PREFIX := $(shell brew --prefix) |
There was a problem hiding this comment.
😂 Well, that's one solution, I guess
24 tasks
kartben
approved these changes
Jun 25, 2024
tomi-font
approved these changes
Jun 25, 2024
| @@ -3,6 +3,8 @@ | |||
|
|
|||
| .PHONY: all clean | |||
|
|
|||
| OS = $(shell uname -s) | |||
There was a problem hiding this comment.
Suggested change
| OS = $(shell uname -s) | |
| OS := $(shell uname -s) |
| @@ -3,6 +3,8 @@ | |||
|
|
|||
| .PHONY: all clean | |||
|
|
|||
| OS = $(shell uname -s) | |||
There was a problem hiding this comment.
Suggested change
| OS = $(shell uname -s) | |
| OS := $(shell uname -s) |
| .. group-tab:: macOS | ||
|
|
||
| .. code-block:: bash | ||
| :caption: Install thrift dependencies in macOS |
There was a problem hiding this comment.
Suggested change
| :caption: Install thrift dependencies in macOS | |
| :caption: Install thrift dependencies on macOS |
There was a problem hiding this comment.
Hmm.. do people normally say a package is installed "on" an OS? I usually say "in".
It's possibly "on" in German or French, but I've been saying "in" for the last 20 years or so and I'm a native English speaker.
There was a problem hiding this comment.
Oh. Could well be that you're right then. I say "on" but I'm not a native English speaker.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added tabs to the thrift doc so that building works for macos and is consistent with the rest of the documentation in terms of one tab per os (I unfortunately do not have a windows dev environment at my disposal, but would be happy to add those package installation instructions if someone knows what they might be).
Updated client and server makefiles to discover include paths for boost and openssl.
Doc Preview