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

Feature flag #336

Merged
merged 2 commits into from
Jul 8, 2016
Merged

Feature flag #336

merged 2 commits into from
Jul 8, 2016

Conversation

ghost
Copy link

@ghost ghost commented Jul 8, 2016

-timeout flap to support stopping the gortool after n seconds.

@buger
Copy link
Owner

buger commented Jul 8, 2016

Hello! This is a nice feature, and I would love to merge it. However, there are few changes I would like you to make:

  • Put flag definition to settings.go and Settings object. Instead of flag.Int use flag.DurationVar. This will allow you to specify duration with units: 1s, 30m, 2h etc.
  • We already have http timeout option, and it is not really timeout, make sense rename it to: exit-delay or exit-after.

verbose bool
debug bool
stats bool
exit_after time.Duration
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't use underscores in Go names; struct field exit_after should be exitAfter

@ortz ortz force-pushed the feature/timeout-flag branch from dc3b294 to 776bb28 Compare July 8, 2016 16:00
@ortz
Copy link
Contributor

ortz commented Jul 8, 2016

Hey @buger, thank for the quick response.
I've modified the code and pushed the changes.

Thanks,

@ortz ortz force-pushed the feature/timeout-flag branch from 776bb28 to 6620d25 Compare July 8, 2016 16:04
@buger
Copy link
Owner

buger commented Jul 8, 2016

Do you know why pull requests created from "ghost" user?:)

@ortz
Copy link
Contributor

ortz commented Jul 8, 2016

Yes, I forked and created the PR with my old user, after I realized that I transferred the repository and closed the old account.
This "ghost" is a deleted user, I guess every closed user in Github is a ghost.

@buger buger merged commit 6620d25 into buger:master Jul 8, 2016
@buger
Copy link
Owner

buger commented Jul 8, 2016

Merged, thank you!

@ortz
Copy link
Contributor

ortz commented Jul 8, 2016

Great, thanks! waiting for the next release :)

@ortz ortz deleted the feature/timeout-flag branch July 8, 2016 16:31
@ortz ortz restored the feature/timeout-flag branch July 8, 2016 16:31
@buger
Copy link
Owner

buger commented Aug 11, 2016

New version v0.15.0 include this features. Thanks!

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.

3 participants