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

fix: capture_benchmark function fix for component downloads #4652

Merged
merged 3 commits into from
Jul 17, 2024

Conversation

zachary-bailey
Copy link
Collaborator

What type of PR is this?

/kind bug

What this PR does / why we need it:

This PR enables the capture_benchmark function to handle arguments that contain whitespaces and hyphens. The code change results in these characters being converted into underscores and the function successfully continuing.

Which issue(s) this PR fixes:

Benchmarks for the install-dependencies.sh script are not currently available because some new variable names contain hyphens which is breaking the function.

Requirements:

@zachary-bailey zachary-bailey self-assigned this Jul 15, 2024
@zachary-bailey zachary-bailey changed the title fix: capture_benchmark function fix for plugin downloads fix: capture_benchmark function fix for component downloads Jul 15, 2024
@coveralls
Copy link

coveralls commented Jul 15, 2024

Pull Request Test Coverage Report for Build 9974903105

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 71.021%

Totals Coverage Status
Change from base Build 9972041896: 0.0%
Covered Lines: 2566
Relevant Lines: 3613

💛 - Coveralls

@@ -413,7 +413,10 @@ check_array_size() {

capture_benchmark() {
set +x
benchmarks+=($1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

for future reference, let's avoid using parameters directly within logic, e.g. always assign to a local variable to give the parameter a meaningful name before using elsewhere within the function/script.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do!

@zachary-bailey zachary-bailey merged commit 4f54630 into master Jul 17, 2024
19 checks passed
@zachary-bailey zachary-bailey deleted the zacharybailey/benchmarksFixForPlugins branch July 17, 2024 14:30
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.

None yet

4 participants