-
Notifications
You must be signed in to change notification settings - Fork 54
Sed for linux #250
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
Sed for linux #250
Conversation
ecc8cf5 to
a17ce00
Compare
ckadner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Phu. This is great. We need to make a few more changes though.
- We need to exclude any
*.bundle.jsfiles. These are generated and meant to be as small as possible - We need different replacements for the various file types since comments in Bash scripts are different from comments in HTML or CSS. So we should instead create one
aliasfor thesedcommand, i.e. call itgsedat the beginning of the script and then use that and leave thehash_commentandslash_comment, ... functions unchanged otherwise.
if [[ "$OSTYPE" == "linux-gnu"* ]]; then # Linux
alias gsed="sed -i"
elif [[ "$OSTYPE" == "darwin"* ]]; then # macOS
alias gsed="sed -i ''"
elif [[ "$OSTYPE" == "cygwin" ]]; then # POSIX compatible emulation for Windows
alias gsed="sed -i"
else
echo "FAILED. OS not compatible with script '/hack/add_license_headers.sh'"
exit 1
fiAnd then use gsed in the *_comment () functions:
hash_comment () {
echo "$1"
if ! grep -q "SPDX-License-Identifier" "$1"
then
gsed '1i\
# Copyright 2021 The MLX Contributors\
#\
# SPDX-License-Identifier: Apache-2.0\
' "$1"
fi
}
Signed-off-by: Phu Thai <phuthai450@gmail.com> macos syntax fix Signed-off-by: Phu Thai <phuthai450@gmail.com>
47f981f to
b04cb8a
Compare
Signed-off-by: Phu Thai <phuthai450@gmail.com> removed bundle license ignore *.bundle.js Signed-off-by: Phu Thai <phuthai450@gmail.com>
b04cb8a to
1a0afba
Compare
ckadner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Phu just a few more changes :-)
1a0afba to
f831b9e
Compare
ckadner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @BluThaitanium.
I just gave it a spin and came across a few issues.
Signed-off-by: Phu Thai <phuthai450@gmail.com> PR comments
f831b9e to
2a7e25b
Compare
Signed-off-by: Phu Thai <phuthai450@gmail.com>
2a7e25b to
b03d6d5
Compare
ckadner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Thank you @BluThaitanium
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BluThaitanium, ckadner The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes updating licensing by running the correct sed command corresponding to Linux and MacOS
For readability- Sed commands are in separate functions, called by each
xxxx_comment()function in add_license_headers.sh during execution.Throws error statement without exit code, indicating the command isn't working due to OS limitations of sed (i.e. user is using Windows)
Resolves #249
@ckadner