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

Added parser options: DefaultDuration #15

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aubelsb2
Copy link
Contributor

@aubelsb2 aubelsb2 commented Aug 9, 2021

I've added a defautl duration as required for my usecase.

It changes the signature to the Parse function but by using vargs it shouldn't impact any existing usage. I have extended the test cases to match too.

A futher extension I would consider for my use-case is: DisableCustomDuration to force the default duration. (Or perhaps it would be "ForceDuration?")

I haven't found a "great" way of doing opt args for options. This is typically how I do it. I have also seen people use it as an internal struct modifier ie:

type Opts struct {
 defautlDuration *time.Duration
}

type ParserOpt interface {
  Apply(o *Opts) error
}

type DefaultDuration time.Duration

func (dd DefaultDuration) Apply(o *Opts) error {
  var t time.Duration = time.Duration(dd)
  o.defaultDuration = &t
  return nil 
}

...
for _ opt := range opts { opt.apply(&opts) }
...

@aubelsb2
Copy link
Contributor Author

Fixed other deepsource issues in other PR. #17 if both merged there should be no issue

@aubelsb2
Copy link
Contributor Author

Merge conflict resolved.

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.

1 participant