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

random updates #17

Merged
merged 3 commits into from
Mar 2, 2019
Merged

random updates #17

merged 3 commits into from
Mar 2, 2019

Conversation

mreiferson
Copy link
Owner

see commit message

@ploxiln
Copy link
Contributor

ploxiln commented Mar 1, 2019

it looks like, with this change, the coerce functions don't need to handle their own type anymore? e.g.

func coerceDuration(v interface{}) (time.Duration, error) {
	...
	case time.Duration:
		return v.(time.Duration), nil

@ploxiln
Copy link
Contributor

ploxiln commented Mar 1, 2019

you could update to the go versions in the travis test matrix, probably fine to add to this branch

(and I wonder if you could/should manually back-date the go version in the go.mod file)

@ploxiln
Copy link
Contributor

ploxiln commented Mar 1, 2019

looks great to me

@mreiferson
Copy link
Owner Author

check that last commit @ploxiln, the int* cases need to still handle all varieties...

* use Get() rather than String() for flag values
* don't coerce values of the same type (enables support for custom flag
  types)
* remove nsq-specific (deprecated) Duration handling
@ploxiln
Copy link
Contributor

ploxiln commented Mar 2, 2019

Yeah, removing self-support was a possible silly idea, sorry about that ... I see it's needed when you convert an int64 to a uint16 by going through coerceInt64() first, or when you convert a float64 to a float32 by going through coerceFloat64() first.

Anyway, 👍

@mreiferson mreiferson merged commit 20ba7d3 into master Mar 2, 2019
@mreiferson mreiferson deleted the updates branch March 2, 2019 06:49
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.

2 participants