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

Add timeout option #9

Merged
merged 1 commit into from Jan 21, 2020
Merged

Add timeout option #9

merged 1 commit into from Jan 21, 2020

Conversation

lutfuahmet
Copy link
Contributor

@lutfuahmet lutfuahmet commented Jan 18, 2020

default timeout is 5 seconds. I added t, timeout flag to cli and Timeout field to struct.

Copy link
Owner

@rogerwelin rogerwelin left a comment

Choose a reason for hiding this comment

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

Hi @lutfuahmet, thanks for the PR! There are three of things I would like you to fix before we merge this PR:

  • run go fmt on all the files you made changes to
  • Do a git squash the 5 commits into one
  • The if else statements (in cli.go) on the c.Int("timeout") added are not needed, since you added a default value of 5 in the IntFlag we can just add c.Int("timeout") to the Timeout field of the Cassowary struct directly without the if/else

Then push those changes and we'll merge them. Again, thanks for the PR!

@lutfuahmet
Copy link
Contributor Author

Hi @lutfuahmet, thanks for the PR! There are three of things I would like you to fix before we merge this PR:

  • run go fmt on all the files you made changes to
  • Do a git squash the 5 commits into one
  • The if else statements (in cli.go) on the c.Int("timeout") added are not needed, since you added a default value of 5 in the IntFlag we can just add c.Int("timeout") to the Timeout field of the Cassowary struct directly without the if/else

Then push those changes and we'll merge them. Again, thanks for the PR!

I fixed them.

@rogerwelin
Copy link
Owner

rogerwelin commented Jan 21, 2020

Hi @lutfuahmet, thanks for the PR! There are three of things I would like you to fix before we merge this PR:

  • run go fmt on all the files you made changes to
  • Do a git squash the 5 commits into one
  • The if else statements (in cli.go) on the c.Int("timeout") added are not needed, since you added a default value of 5 in the IntFlag we can just add c.Int("timeout") to the Timeout field of the Cassowary struct directly without the if/else

Then push those changes and we'll merge them. Again, thanks for the PR!

I fixed them.

Good job! Just one minor fix, could you please remove this line (and the log import) in main.go:

log.SetFlags(log.LstdFlags | log.Lshortfile)

@lutfuahmet
Copy link
Contributor Author

Hi @lutfuahmet, thanks for the PR! There are three of things I would like you to fix before we merge this PR:

  • run go fmt on all the files you made changes to
  • Do a git squash the 5 commits into one
  • The if else statements (in cli.go) on the c.Int("timeout") added are not needed, since you added a default value of 5 in the IntFlag we can just add c.Int("timeout") to the Timeout field of the Cassowary struct directly without the if/else

Then push those changes and we'll merge them. Again, thanks for the PR!

I fixed them.

Good job! Just one minor fix, could you please remove this line (and the log import) in main.go:

log.SetFlags(log.LstdFlags | log.Lshortfile)

I removed.

@rogerwelin rogerwelin merged commit 13d53ad into rogerwelin:master Jan 21, 2020
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.

None yet

2 participants