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

scripts(compute_branch): fix incorrect conditional #4987

Merged

Conversation

algolucky
Copy link
Contributor

@algolucky algolucky commented Jan 9, 2023

Summary

this was introduced in #4927

  • updates the conditional in compute_branch.sh to use the -z operator, otherwise it works opposite as it was intended.

Test Plan

  • ci
  • locally (see below)

Executing make build you should see the -X github.com/algorand/go-algorand/config.Branch=fix/compute-branch-condition

image

this is what we see on rel/nightly (it's empty)

image

@algolucky algolucky self-assigned this Jan 9, 2023
@algolucky algolucky marked this pull request as ready for review January 9, 2023 19:12
@algolucky algolucky requested a review from algorandskiy January 9, 2023 19:15
@algolucky algolucky changed the title fix(scripts): use -z in conditional scripts(compute_branch): fix incorrect conditional Jan 9, 2023
onetechnical
onetechnical previously approved these changes Jan 9, 2023
Copy link
Contributor

@onetechnical onetechnical left a comment

Choose a reason for hiding this comment

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

This seems to work from my testing.

Copy link
Contributor

@algorandskiy algorandskiy left a comment

Choose a reason for hiding this comment

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

Please rebase on top of master, TestREDCongestionManagerShouldntDrop fixed in #4986

@codecov
Copy link

codecov bot commented Jan 9, 2023

Codecov Report

Merging #4987 (14dc5e1) into master (14dc5e1) will not change coverage.
The diff coverage is n/a.

❗ Current head 14dc5e1 differs from pull request most recent head 5988867. Consider uploading reports for the commit 5988867 to get more accurate results

@@           Coverage Diff           @@
##           master    #4987   +/-   ##
=======================================
  Coverage   53.62%   53.62%           
=======================================
  Files         432      432           
  Lines       54048    54048           
=======================================
  Hits        28985    28985           
  Misses      22814    22814           
  Partials     2249     2249           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@algorandskiy algorandskiy merged commit 5689093 into algorand:master Jan 9, 2023
@algolucky algolucky deleted the fix/compute-branch-condition branch January 9, 2023 21:14
winder pushed a commit to winder/go-algorand that referenced this pull request Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants