Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix build script for the builder image #5189

Merged
merged 1 commit into from
Mar 28, 2022
Merged

Fix build script for the builder image #5189

merged 1 commit into from
Mar 28, 2022

Conversation

chevdor
Copy link
Contributor

@chevdor chevdor commented Mar 23, 2022

Allows handling versions with dash such as v1.2.3-1.

fix #4743

@chevdor chevdor added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Mar 23, 2022
@@ -8,17 +8,20 @@ PROJECT_ROOT=`git rev-parse --show-toplevel`
cd $PROJECT_ROOT

# Find the current version from Cargo.toml
VERSION=`grep "^version" ./cli/Cargo.toml | egrep -o "([0-9\.]+)"`
VERSION=`grep "^version" ./cli/Cargo.toml | egrep -o "([0-9\.]+-?[0-9]+)"`
Copy link
Contributor

@coderobe coderobe Mar 23, 2022

Choose a reason for hiding this comment

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

i would do something like this instead:

Suggested change
VERSION=`grep "^version" ./cli/Cargo.toml | egrep -o "([0-9\.]+-?[0-9]+)"`
VERSION=`grep "^version" ./cli/Cargo.toml | egrep -o "([0-9\.\-]+)"`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why that ? Do you expect some 0.9-1.13 versions or something looking like that ?

Copy link
Contributor

Choose a reason for hiding this comment

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

🤷‍♀️ Not necessarily, but shorter regexes are nicer for maintainability IMO.
Fine with either, really

@chevdor chevdor added A0-please_review Pull request needs code review. and removed A0-please_review Pull request needs code review. labels Mar 23, 2022
@chevdor chevdor merged commit 22967a4 into master Mar 28, 2022
@chevdor chevdor deleted the wk-fix-4743 branch March 28, 2022 07:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix Docker build script
2 participants