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

HOTFIX Dont omit location database on --location.database="" #165

Merged
merged 2 commits into from Feb 19, 2018

Conversation

Projects
None yet
3 participants
@Waldz
Copy link
Member

commented Feb 15, 2018

No description provided.

@Waldz Waldz requested review from tadovas, donce and zolia Feb 15, 2018

} else {
locationDetector = location.NewDetectorFake("")
locationDetector = location.NewDetector(filepath.Join(options.DirectoryConfig, options.LocationDatabase))

This comment has been minimized.

Copy link
@donce

donce Feb 15, 2018

Contributor

Mutating options.LocationDatabase (in options.go) is bad - this makes it hard to understand in later code, why options.LocationDatabase is assumed to be present, even though this option is optional.
To solve this, we can use default location detabase here:

...
else {
  var locationDatabase string
  if options.DirectoryConfig != "" {
    locationDatabase = options.DirectoryConfig
  } else {
    locationDatabase = defaultLocationDatabase
  }
  locationDetector = location.NewDetector(filepath.Join(locationDatabase, options.LocationDatabase))
}

This comment has been minimized.

Copy link
@Waldz

Waldz Feb 15, 2018

Author Member

Fixed

@Waldz Waldz changed the title HOTFOX Dont omit location database on --location.database="" HOTFIX Dont omit location database on --location.database="" Feb 15, 2018

@Waldz Waldz force-pushed the hotfix/location-database branch from 9f6d7a9 to 797af16 Feb 15, 2018

@donce

donce approved these changes Feb 15, 2018

@zolia

zolia approved these changes Feb 19, 2018

@zolia zolia merged commit d4827e2 into master Feb 19, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@Waldz Waldz deleted the hotfix/location-database branch Feb 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.