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

Initialize variable as empty if not passed; fixes #2876 #2878

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mbentley
Copy link
Member

@mbentley mbentley commented Jul 8, 2022

Signed-off-by: Matt Bentley mbentley@vmware.com

What this PR does / why we need it

Addresses unexpected behavior in airgap script

Which issue(s) this PR fixes

Fixes #2876

Describe testing done for PR

Validated that the script runs with the change.

Release note

n/a - not really release note worthy


PR Checklist

  • Squash the commits into one or a small number of logical commits
  • Use good commit messages
  • Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

Signed-off-by: Matt Bentley <mbentley@vmware.com>
@mbentley mbentley requested a review from a team as a code owner July 8, 2022 13:09
@codecov
Copy link

codecov bot commented Jul 8, 2022

Codecov Report

Merging #2878 (51fb941) into main (cb46355) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2878   +/-   ##
=======================================
  Coverage   43.82%   43.83%           
=======================================
  Files         414      414           
  Lines       41295    41295           
=======================================
+ Hits        18097    18101    +4     
+ Misses      21503    21500    -3     
+ Partials     1695     1694    -1     
Impacted Files Coverage Δ
addons/controllers/clusterbootstrap_controller.go 63.90% <0.00%> (+0.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb46355...51fb941. Read the comment docs.

Copy link
Contributor

@vuil vuil left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for submitting the issue and fixing it!

@vuil vuil added area/ux UX related ok-to-merge PRs should be labelled with this before merging labels Jul 19, 2022
@vuil
Copy link
Contributor

vuil commented Jul 19, 2022

@mbentley please see the link in the vmwclabot check for details on signing a CLA as a contributor to vmware-tanzu projects like this one.

@mbentley
Copy link
Member Author

mbentley commented Jul 19, 2022

@mbentley please see the link in the vmwclabot check for details on signing a CLA as a contributor to vmware-tanzu projects like this one.

As a VMware employee, I've gone through the steps suggested when I originally opened #2877 and am now a member of the GitHub org. Anything else I am needing to do? Also checking internally to see if anyone might have any ideas why the @vmwclabot is asking for the CLA to be signed when I am a member of the vmware-tanzu org.

@mbentley
Copy link
Member Author

mbentley commented Aug 3, 2022

@vuil - looks like everything should be OK from a merging standpoint; someone was able to fix the CLA issue.

@jmoroski jmoroski removed the ok-to-merge PRs should be labelled with this before merging label Apr 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updates to hack/gen-publish-images.sh don't take into account not needing a custom CA
4 participants