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

chore: add support for duration format for timeout flag #488

Merged
merged 5 commits into from
Feb 2, 2023

Conversation

yassinouider
Copy link
Contributor

The 'timeout' flag of the test command now accepts human-readable duration format, such as '0.5s' or '1s' etc, in addition to the previous format of 'ns'. This change makes it more convenient for users to specify the execution time of the test command.

This change includes the following updates:

  • In function applyFlagToFieldReflectString, added check for type time.Duration
  • Added test cases in flags_test.go to check the implementation of the above change
  • Fixed the comment in struct definition to indicate the support for duration format in timeout flag
  • Remove FIXME comment in timeout flag parsing function

@yassinouider yassinouider requested a review from a team as a code owner January 27, 2023 11:54
cmd/gnodev/test.go Outdated Show resolved Hide resolved
pkgs/command/flags.go Outdated Show resolved Hide resolved
Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

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

Looks good 💯

Thank you for the contribution 🙏

I'm not a huge fan of using the reflect package, especially for something as simple as flag parsing, hopefully we can do away with that in #497

pkgs/command/flags_test.go Show resolved Hide resolved
@moul moul changed the title Add support for duration format for timeout flag chore: add support for duration format for timeout flag Feb 2, 2023
@moul moul merged commit bd13cf7 into gnolang:master Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants