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

Fix bug with unspecified HTTPClient #1938

Merged
merged 1 commit into from
Oct 28, 2021
Merged

Fix bug with unspecified HTTPClient #1938

merged 1 commit into from
Oct 28, 2021

Conversation

matthchr
Copy link
Member

Horrible

@matthchr matthchr added this to the codegen-alpha-3 milestone Oct 28, 2021
babbageclunk
babbageclunk approved these changes Oct 28, 2021
View reviewed changes
v2/internal/genericarmclient/generic_client.go
// We assign this HTTPClient like this because if we actually set it to nil, due to the way
// go interfaces wrap values, the subsequent if httpClient == nil check returns false (even though
// the value IN the interface IS nil).
if httpClient != nil {
Copy link
Member

Choose a reason for hiding this comment

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

😢

theunrepentantgeek
theunrepentantgeek approved these changes Oct 28, 2021
View reviewed changes
Copy link
Member

@theunrepentantgeek theunrepentantgeek left a comment

Choose a reason for hiding this comment

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

Looks good.

gif

@codecov-commenter
Copy link

Codecov Report

Merging #1938 (7ab46c8) into main (2895dcb) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1938      +/-   ##
==========================================
+ Coverage   56.64%   56.66%   +0.02%     
==========================================
  Files         375      375              
  Lines       70284    70285       +1     
==========================================
+ Hits        39811    39829      +18     
+ Misses      25582    25561      -21     
- Partials     4891     4895       +4
Impacted Files Coverage Δ
v2/internal/genericarmclient/generic_client.go 70.77% <100.00%> (+0.19%) ⬆️
...ternal/reconcilers/azure_generic_arm_reconciler.go 76.69% <0.00%> (-0.49%) ⬇️
...e/v1alpha1api20210501/managed_cluster_types_gen.go 51.80% <0.00%> (-0.08%) ⬇️
...internal/reconcilers/fatal_reconciliation_error.go 100.00% <0.00%> (+33.33%) ⬆️
...ernal/testcommon/error_translating_roundtripper.go 56.25% <0.00%> (+43.75%) ⬆️

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 2895dcb...7ab46c8. Read the comment docs.

@matthchr matthchr merged commit 7720077 into Azure:main Oct 28, 2021
@matthchr matthchr deleted the feature/fix-bug branch October 28, 2021 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theunrepentantgeek theunrepentantgeek theunrepentantgeek approved these changes

@babbageclunk babbageclunk babbageclunk approved these changes

@davefellows davefellows Awaiting requested review from davefellows davefellows is a code owner

@Porges Porges Awaiting requested review from Porges

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
codegen-alpha-3
Development

Successfully merging this pull request may close these issues.
4 participants
@matthchr @codecov-commenter @babbageclunk @theunrepentantgeek