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
Merged

Conversation

@vschettino
Copy link
Contributor

@vschettino vschettino commented Jul 11, 2020

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 urfave/cli as a code owner Jul 11, 2020
@vschettino vschettino requested review from saschagrunert and lynncyrin and removed request for urfave/cli Jul 11, 2020
Copy link
Member

@saschagrunert saschagrunert left a comment

LGTM

@rliebz
rliebz approved these changes Jul 12, 2020
@rliebz rliebz merged commit d2d2098 into urfave:master Jul 12, 2020
12 checks passed
12 checks passed
ubuntu-latest @ Go 1.12
Details
ubuntu-latest @ Go 1.13
Details
ubuntu-latest @ Go 1.14
Details
macos-latest @ Go 1.12
Details
macos-latest @ Go 1.13
Details
macos-latest @ Go 1.14
Details
windows-latest @ Go 1.12
Details
windows-latest @ Go 1.13
Details
windows-latest @ Go 1.14
Details
test-docs
Details
codecov/patch 100.00% of diff hit (target 73.27%)
Details
codecov/project 73.37% (+0.09%) compared to 8e16b98
Details
@xpol
Copy link

@xpol xpol commented Aug 14, 2020

Would there be a new release for this fix?

@lynncyrin
Copy link
Member

@lynncyrin lynncyrin commented Aug 17, 2020

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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants