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

feature: update install.sh version regex #143

Merged
merged 2 commits into from
Apr 13, 2022
Merged

Conversation

Alexxxing
Copy link
Contributor

feature: update install.sh version regex

新版本v0.10.0无法匹配到
@wu-sheng wu-sheng requested review from fgksgf and kezhenxu94 April 13, 2022 07:18
@@ -51,7 +51,7 @@ done
OS=$(echo $OS | awk '{print tolower($0)}')

# Get the latest version of swctl.
VERSION=$(curl "https://raw.githubusercontent.com/apache/skywalking-website/master/data/releases.yml" | grep --after-context=7 "name: SkyWalking CLI" | grep "version" | grep -o "[0-9].[0-9].[0-9]")
VERSION=$(curl "https://raw.githubusercontent.com/apache/skywalking-website/master/data/releases.yml" | grep --after-context=7 "name: SkyWalking CLI" | grep "version" | grep -Eo "[0-9].[0-9]+.[0-9]")
Copy link
Member

Choose a reason for hiding this comment

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

You might be using GNU grep so it supports regex by default, but -E is needed in non-GNU grep. What is the problem in your case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't install skywalking-cli on Centos 7.x,the match result is None,i need to modify the install.sh regex to install skywalking-cli v0.10.0

Copy link
Member

Choose a reason for hiding this comment

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

I can't install skywalking-cli on Centos 7.x,the match result is None,i need to modify the install.sh regex to install skywalking-cli v0.10.0

OK sorry I missed one point in your changed file, "[0-9].[0-9].[0-9]" --> "[0-9].[0-9]+.[0-9]" is totally right, I meant modification from -o to -Eo is not necessary ?

Copy link
Member

Choose a reason for hiding this comment

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

Also I'd suggest just write the regex as "[0-9]+.[0-9]+.[0-9]+", this is future-proof so that we won't be error if there is versions like 11.11.11

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's right, i just point out one of the problems. and here you need to specify -E on CentOS,it‘s my test result

Copy link
Member

Choose a reason for hiding this comment

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

@Alexxxing Thanks!!

update install.sh regex to match new version
@wu-sheng wu-sheng added this to the 0.11.0 milestone Apr 13, 2022
@wu-sheng wu-sheng added the bug Something isn't working label Apr 13, 2022
@wu-sheng wu-sheng merged commit 476ff7a into apache:master Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants