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

Add suggestions for slices and array element missing types #390

Merged
merged 3 commits into from
Aug 10, 2023
Merged

Add suggestions for slices and array element missing types #390

merged 3 commits into from
Aug 10, 2023

Conversation

paullen
Copy link
Contributor

@paullen paullen commented Jun 8, 2023

Changes:

Added suggestions for requesting slice/array of pointers instead of the elements and vice versa

@CLAassistant
Copy link

CLAassistant commented Jun 8, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@paullen
Copy link
Contributor Author

paullen commented Jun 23, 2023

Hey. Just wanted to follow up on this PR. This seems like a good suggestion to have, it is a part of the solution for Issue #184 . Then there is the issue #389 that addresses the other half of the #184 issue.

@JacobOaks
Copy link
Contributor

JacobOaks commented Jul 12, 2023 •
edited
Loading

Hey @paullen, thanks for this PR!

I think this is a good addition, can we just add a test case similar to this one that tests that the suggestions get properly populated into an error from a call to fx.Invoke?

@paullen
Copy link
Contributor Author

paullen commented Jul 22, 2023

@JacobOaks Added test cases, the test coverage is now 99%, up from 98.3%. Please review. Thanks!

@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #390 (a5d35dc) into master (a30081d) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #390      +/-   ##
==========================================
+ Coverage   98.39%   98.40%   +0.01%     
==========================================
  Files          22       22              
  Lines        1492     1502      +10     
==========================================
+ Hits         1468     1478      +10     
  Misses         15       15              
  Partials        9        9
Files Changed Coverage Δ
error.go 100.00% <100.00%> (ø)

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

JacobOaks
JacobOaks approved these changes Aug 8, 2023
View reviewed changes
Copy link
Contributor

@JacobOaks JacobOaks left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hey, sorry for the delay. LGTM! Thanks for the PR.

sywhang
sywhang approved these changes Aug 10, 2023
View reviewed changes
Copy link
Contributor

@sywhang sywhang left a comment

Choose a reason for hiding this comment

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

LGTM.

@sywhang sywhang merged commit fc364ac into uber-go:master Aug 10, 2023
4 checks passed
alexisvisco pushed a commit to alexisvisco/dig that referenced this pull request Aug 31, 2023
@paullen @alexisvisco
Add suggestions for slices and array element missing types (uber-go#390)
06bbba5 
* Add suggestions for slices and array element missing types

* Add more test cases

* fix: add test coverage for slice and array of values and pointers suggestions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sywhang sywhang sywhang approved these changes

@JacobOaks JacobOaks JacobOaks approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@paullen @CLAassistant @JacobOaks @sywhang