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

Modify usage of sed to be compatible with macOS #30

Merged
merged 4 commits into from
May 14, 2018

Conversation

KingAkeem
Copy link
Contributor

Mac OS uses a different sed command from GNU. In order to use the -i flag you must pass the extension along with it so I added the .hpp extension if darwin is detected as the OS.

@codecov
Copy link

codecov bot commented May 11, 2018

Codecov Report

Merging #30 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #30   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          13     13           
  Lines         536    536           
=====================================
  Hits          536    536

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69dd72f...c32d115. Read the comment docs.

@faheel faheel force-pushed the master branch 2 times, most recently from 6bf4812 to 69dd72f Compare May 14, 2018 13:38
@faheel
Copy link
Owner

faheel commented May 14, 2018

Good find! But instead of using a modified command to make it work on a different OS, I think we should use those commands that work the same on multiple OSs.

So I suggest you change the sed -i ... line to the following:

sed "/#include \"*\"/d" "$release_file" > "$release_file.tmp"
mv "$release_file.tmp" "$release_file"

@faheel faheel changed the title Added a few lines to release script to allow make release on Mac OS X Modify usage of sed to be compatible with macOS May 14, 2018
@@ -47,5 +47,5 @@ do
printf "\n\n" >> "$release_file"
done

# delete includes for non-standard header files from the release file
Copy link
Owner

Choose a reason for hiding this comment

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

This comment is important 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry about that, adding it back now.

@faheel faheel merged commit be54071 into faheel:master May 14, 2018
@faheel
Copy link
Owner

faheel commented May 14, 2018

Thank you for contributing! 👍

@KingAkeem KingAkeem deleted the macOSFix branch May 14, 2018 16: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.

2 participants