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

support type-safe return values #11

Merged
merged 4 commits into from
Jul 6, 2023
Merged

support type-safe return values #11

merged 4 commits into from
Jul 6, 2023

Conversation

tra4less
Copy link
Contributor

@tra4less tra4less commented Jun 28, 2023 •
edited
Loading

@CLAassistant
Copy link

CLAassistant commented Jun 28, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@trim21
Copy link

trim21 commented Jun 28, 2023

can we finally have this feature 😭

@sywhang sywhang self-assigned this Jun 28, 2023
@sywhang sywhang added the enhancement New feature or request label Jun 28, 2023
@sywhang
Copy link
Contributor

sywhang commented Jun 29, 2023

Thank you for submitting this PR. I'll try to get some time to go over this some time later this week or early next week.

High level thoughts - My vote is 👍 for this feature - we've been using a different mocking library internally at Uber that supports type safe return values, and lack of this feature has been a big headache for us as well.

Preventing breaking changes through CLI flag also looks good to me.

@sywhang
Copy link
Contributor

sywhang commented Jun 29, 2023

Also cc @linzhp and @r-hang

sywhang
sywhang approved these changes Jul 5, 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.

Thank you for the work! Left couple of comments for readability, otherwise looks good to me.

mockgen/mockgen.go Outdated Show resolved Hide resolved
mockgen/mockgen.go Outdated Show resolved Hide resolved
mockgen/mockgen.go Outdated Show resolved Hide resolved
tra4less and others added 3 commits July 6, 2023 11:03
@tra4less
Copy link
Contributor Author

tra4less commented Jul 6, 2023

@sywhang thanks, your advice is effective.

@tra4less tra4less requested a review from trim21 July 6, 2023 03:48
trim21
trim21 reviewed Jul 6, 2023
View reviewed changes
Copy link

@trim21 trim21 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 b9b5dc7 into uber-go:main Jul 6, 2023
@tra4less
Copy link
Contributor Author

tra4less commented Jul 6, 2023

Could you please tag this PR? Thank you! @sywhang

@sywhang
Copy link
Contributor

sywhang commented Jul 6, 2023

yup, I'll be tagging a version with this along with a few other changes some time this week. Thanks.

@tra4less
Copy link
Contributor Author

tra4less commented Jul 6, 2023

yup, I'll be tagging a version with this along with a few other changes some time this week. Thanks.

get

@sywhang
Copy link
Contributor

sywhang commented Jul 6, 2023

@n0trace v0.2.0 is tagged.

@fasmat fasmat mentioned this pull request Jul 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trim21 trim21 trim21 left review comments

@sywhang sywhang sywhang approved these changes

Assignees

@sywhang sywhang

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tra4less @CLAassistant @trim21 @sywhang