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.

Sign up for GitHub

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

Jump to bottom

Release Candidate 2023.2.3 #4458

Merged
merged 1 commit into from
Jun 29, 2023
Merged

Release Candidate 2023.2.3 #4458

merged 1 commit into from
Jun 29, 2023

Conversation

ONECasey
Copy link
Contributor

@MaxMustermann2
scripts: restore install_build_tools.sh (#4457)
9df6215 
With #4427, the protofiles are generated using a Docker image and thus
`protoc` related binaries are not required. However, `mockgen` and
`golangci-lint` are still being used by our Travis build. This change
restores the file that installs them, and removes `protoc` related files
from the list of installations.
@ONECasey ONECasey added the Next Release Use this label for the next release label Jun 28, 2023
@ONECasey ONECasey self-assigned this Jun 28, 2023
@ONECasey
Copy link
Contributor Author

Waiting for hard fork PR from @MaxMustermann2

@ONECasey ONECasey marked this pull request as ready for review June 29, 2023 15:41
@ONECasey ONECasey merged commit 7ad50af into main Jun 29, 2023
GheisMohammadi
GheisMohammadi reviewed Jun 30, 2023
View reviewed changes
scripts/install_build_tools.sh
/*) progdir="/";;
*/*) progdir="${0%/*}";;
*) progdir=".";
esac
Copy link
Contributor

Choose a reason for hiding this comment

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

if the current directory path follows the pattern /* then progdir will be / which gives the wrong path in line 12, because line 12 assumes the / won't be there and it hard-coded the slash.
from line 5 to line 10 could be replaced with this simple command:

progdir="$(dirname "${0}")"

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, I just restored the file that was deleted. Feel free to make edits in a separate PR. Now that the build is working it'll be pretty easy to test. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GheisMohammadi GheisMohammadi GheisMohammadi left review comments

@adsorptionenthalpy adsorptionenthalpy adsorptionenthalpy approved these changes

@sophoah sophoah sophoah approved these changes

@Frozen Frozen Frozen approved these changes

@MaxMustermann2 MaxMustermann2 Awaiting requested review from MaxMustermann2

Assignees

@ONECasey ONECasey

Labels
Next Release Use this label for the next release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ONECasey @Frozen @sophoah @GheisMohammadi @adsorptionenthalpy @MaxMustermann2