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

Allow using Value with TimestampFlag #1160

Merged
merged 2 commits into from Jul 12, 2020

Conversation

vschettino
Copy link
Contributor

What type of PR is this?

  • bug
  • cleanup
  • documentation
  • feature

What this PR does / why we need it:

Allows to use default Value with TimestampFlags.

  • Check if there's a Value before overriding
  • Basic unit test

Which issue(s) this PR fixes:

fixes #1144

Special notes for your reviewer:

Thanks @aloababa for the tip!
cc @lynncyrin

Testing

Just a small unit test to check if setting a Value will led to a filled parameter when there is no cli input.

Release Notes

Fixed TimestampFlag to allow default Value assignment.

@vschettino vschettino requested a review from a team as a code owner July 11, 2020 19:04
@vschettino vschettino requested review from saschagrunert and coilysiren and removed request for a team July 11, 2020 19:04
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

LGTM

@rliebz rliebz merged commit d2d2098 into urfave:master Jul 12, 2020
@xpol
Copy link

xpol commented Aug 14, 2020

Would there be a new release for this fix?

@coilysiren
Copy link
Member

Someone would have to create a new release, I don't know offhand if anyone is currently planning to do so!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v2 bug: Value does not work with TimestampFlag
5 participants