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.

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

Implement debug.List #9163

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mohsalsaleem
Copy link

Refers #9148
This PR implements debug.List method that uses reflect package to to list exported fields and methods names on structs/pointers and all keys in a map.

@CLAassistant
Copy link

CLAassistant commented Nov 12, 2021

CLA assistant check
All committers have signed the CLA.

@bep
Copy link
Member

bep commented Nov 12, 2021

Thanks, can you push the "sign CLA button" above?

@mohsalsaleem
Copy link
Author

I’ve completed the CLA Signing, but for some reason the bot thinks I have not.

@bep
Copy link
Member

bep commented Nov 12, 2021

I’ve completed the CLA Signing, but for some reason the bot thinks I have not.

You have probably a email mismatch between the mail address you used for the commit (as in: your Git user) and the email address registered on GitHub. You can fix this by adding email aliases in your GitHub profile.

@mohsalsaleem
Copy link
Author

Thanks for pointing it out, looks like CLA check passes now!

@mohsalsaleem
Copy link
Author

In case I need to add tests, please let me know. I looked around and could not find anything specific to add tests to apart from the init.go file, which I'm unclear on how to structure the example.

@bep
Copy link
Member

bep commented Nov 12, 2021

In case I need to add tests, please let me know.

We need some basic tests. Just create a debug_test.go file and put a basic TestList test with a simple test structs and a map. The init test you can ignore.

@mohsalsaleem mohsalsaleem marked this pull request as ready for review November 13, 2021 15:35
@mohsalsaleem
Copy link
Author

@bep I just added some basic tests, please have a look! Thanks.

@mohsalsaleem
Copy link
Author

I just noticed that the functions also returns fields that are not exported, let me fix that.

@mohsalsaleem
Copy link
Author

Fixed the issue, please review.
Thanks.

@bep
Copy link
Member

bep commented Nov 21, 2021

Thanks for this. I need to think a little before pulling this into the main branch. I have tested this a little, and while it does work great on technical level, I fear it may expose too much (deprecated and internal methods (not meant to be used in the templates, e.g. MarshalJSON) which I think may be more confusing than useful. I need to think about a way to filter out those.

@bep bep added the Keep label Oct 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants