-
Notifications
You must be signed in to change notification settings - Fork 607
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
1 similar comment
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
1 similar comment
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
g := generator{} | ||
|
||
result := g.getArgNames(testCase.method) | ||
if !testEqSliceStr(t, result, testCase.expected) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend using reflect.DeepEquals
here, but it seems like we have a name collision in that package. I will refactor this later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's what I planned initially, but since there was a conflict of names, I decided not to touch upon it at the moment.
Thank you for your contribution! 🎆 |
The reflect method should be renamed so it does not collide with the reflect package name. As is, it makes import reflect in the code non-idiomatic. Also renamed sourceMode to stay consistent. Noticed this here: #371 Also, refactor slice equals method
Fixes #370 .
Description
Submitter Checklist