Skip to content

support embedded generic interfaces#13

Merged
sywhang merged 3 commits into
mainfrom
unknown repository
Jul 11, 2023
Merged

support embedded generic interfaces#13
sywhang merged 3 commits into
mainfrom
unknown repository

Conversation

@tra4less
Copy link
Copy Markdown
Contributor

@tra4less tra4less commented Jun 28, 2023

@tra4less
Copy link
Copy Markdown
Contributor Author

tra4less commented Jun 29, 2023

cc @sywhang

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

sywhang commented Jun 29, 2023

Thanks @n0trace , I'll get to this shortly.

@bradleygore
Copy link
Copy Markdown

bradleygore commented Jun 30, 2023

Hey @n0trace and @sywhang - I have a similar PR in the origin repo: golang/mock#663
One thing in that PR is that there is some (hopefully good) cleanup around the parse.go file (https://github.com/golang/mock/pull/663/files#diff-eaed3d31482bbbe3987808f6f3e8cf43426d098cd55d0c37699d691fb4da8d85) that I'd love to see make it in if nothing else. I know they just recently flagged the origin repo as archived and such, so I don't want to inundate with PRs especially since the PR in this thread is to solve the same issue 😄

TIA for your consideration, and for taking over this great tool!

r-hang
r-hang reviewed Jul 7, 2023
View reviewed changes
Comment thread mockgen/internal/tests/generics/external.go Outdated
Copy link
Copy Markdown
Contributor

@r-hang r-hang Jul 7, 2023

Choose a reason for hiding this comment

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

One case I think we should cover is variadic type parameters.

Copy link
Copy Markdown
Contributor Author

@tra4less tra4less Jul 9, 2023

Choose a reason for hiding this comment

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

I added to https://github.com/uber/mock/pull/13/files#diff-81e59b1c0a482123d79293bb8f998e0d82f8d2177e83e8cd25608b7dd758ee1aR26

Comment thread mockgen/internal/tests/generics/other/other.go Outdated
Copy link
Copy Markdown
Contributor

@r-hang r-hang Jul 7, 2023

Choose a reason for hiding this comment

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

naming: Could the more sophisticated types be named by their function instead of a number? It took some time for me to organize which cases were tested while reviewing.

Copy link
Copy Markdown
Contributor Author

@tra4less tra4less Jul 9, 2023

Choose a reason for hiding this comment

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

thanks, you are right, i redesigned the case, including the additions under this file

r-hang
r-hang reviewed Jul 10, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@r-hang r-hang left a comment

Choose a reason for hiding this comment

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

Changes look good to me after these comment! Thanks for adding the Variadic case and clearing up the function of the examples.

Comment thread mockgen/internal/tests/generics/external.go Outdated
Comment thread mockgen/internal/tests/generics/source/assert_test.go Outdated
sywhang
sywhang approved these changes Jul 11, 2023
View reviewed changes
Copy link
Copy Markdown
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 - thank you for this work!

@sywhang sywhang merged commit 0181604 into uber-go:main Jul 11, 2023
@tra4less tra4less deleted the issues-668 branch July 12, 2023 04:37
ErfanMomeniii pushed a commit to ErfanMomeniii/mock that referenced this pull request Jul 13, 2023
@tra4less
support embedded generic interfaces (uber-go#13)
fe571d5 
migrate golang/mock#669

fixes golang/mock#668
golang/mock#658
golang/mock#700
@ashb ashb mentioned this pull request Apr 16, 2024
Merged
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

@r-hang r-hang r-hang 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.

[mockgen] Errors creating mocks for embedded generic interfaces

4 participants

@tra4less @sywhang @bradleygore @r-hang