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

Remove reflect calls for doc generation #1259

Merged
merged 4 commits into from Apr 22, 2022

Conversation

dearchap
Copy link
Contributor

@dearchap dearchap commented Mar 27, 2021

What type of PR is this?

  • cleanup

What this PR does / why we need it:

Instead of using reflection to figure out values of fields, the doc generation interface has been extended to include everything needed to stringify a flag. Custom flags need to properly implement the interface to get the valid synopsis for flags

Which issue(s) this PR fixes:

Fixes #631

Special notes for your reviewer:

Since the SliceFlags were type checked in the stringify(Flag) function it makes more sense to move those directly to the individual flags

Testing

go test ./...

Release Notes

DocGenerationFlag has bee extended and all currently defined flag type have been modified to use this interface. This allows doc generation to be customizable without having to resort to reflection


@dearchap dearchap requested a review from a team as a code owner Mar 27, 2021
@dearchap dearchap requested review from saschagrunert and asahasrabuddhe and removed request for a team Mar 27, 2021
@stale
Copy link

stale bot commented Jul 28, 2021

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

@stale stale bot added the status/stale stale due to the age of it's last update label Jul 28, 2021
@stale
Copy link

stale bot commented Aug 28, 2021

Closing this as it has become stale.

@stale stale bot closed this Aug 28, 2021
@dearchap
Copy link
Contributor Author

dearchap commented Aug 28, 2021

@rliebz rliebz reopened this Aug 30, 2021
@stale
Copy link

stale bot commented Aug 30, 2021

This issue or PR has been bumped and is no longer marked as stale! Feel free to bump it again in the future, if it's still relevant.

@stale stale bot removed the status/stale stale due to the age of it's last update label Aug 30, 2021
@meatballhat meatballhat added area/v2 relates to / is being considered for v2 kind/cleanup describes internal cleanup / maintaince labels Apr 21, 2022
@meatballhat meatballhat added this to the Release 2.5.0 milestone Apr 21, 2022
@meatballhat meatballhat changed the base branch from master to main Apr 21, 2022
meatballhat added a commit that referenced this pull request Apr 22, 2022
Remove reflect calls for doc generation (#1259)
@meatballhat meatballhat merged commit d83bb8d into urfave:main Apr 22, 2022
2 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v2 relates to / is being considered for v2 kind/cleanup describes internal cleanup / maintaince
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Eliminate reflection calls in flags
3 participants