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

Doc coverage for paths, extensions inconsistent #268

Open
atc0005 opened this issue Jul 12, 2020 · 2 comments
Open

Doc coverage for paths, extensions inconsistent #268

atc0005 opened this issue Jul 12, 2020 · 2 comments
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation
Milestone

Comments

@atc0005
Copy link
Owner

atc0005 commented Jul 12, 2020

README says:

  • extensions

    • mentions that the default is an empty list
  • paths

    • mentions N/A for the default list

Code says:

Paths []string `toml:"paths" arg:"--paths,env:ELBOW_PATHS" help:"List of comma or space-separated paths to process."`

FileExtensions []string `toml:"file_extensions" arg:"--extensions,env:ELBOW_EXTENSIONS" help:"Limit search to specified file extensions. Specify as space separated list to match multiple required extensions."`


elbow/config/get.go

Lines 134 to 141 in 6945786

// GetPaths returns the paths field if it's non-nil, app default value
// otherwise
func (c *Config) GetPaths() []string {
if c == nil || c.Paths == nil {
return nil
}
return c.Paths
}

elbow/config/get.go

Lines 73 to 87 in 6945786

// GetFileExtensions returns the fileExtensions field if it's non-nil, zero value
// otherwise.
// TODO: Double check this one; how should we safely handle returning an
// empty/zero value?
// As an example, the https://github.com/google/go-github package has a
// `Issue.GetAssignees()` method that returns nil if the `Issue.Assignees`
// field is nil. This seems to suggest that this is all we really can do here?
//
func (c *Config) GetFileExtensions() []string {
if c == nil || c.FileExtensions == nil {
// FIXME: Isn't the goal to avoid returning nil?
return nil
}
return c.FileExtensions
}

@atc0005 atc0005 added the documentation Improvements or additions to documentation label Jul 12, 2020
@atc0005 atc0005 added this to the Future milestone Jul 12, 2020
@atc0005 atc0005 self-assigned this Jul 12, 2020
@atc0005
Copy link
Owner Author

atc0005 commented Jul 12, 2020

This will probably be a lot clearer once I refactor the config handling to match more recent projects such as atc0005/dnsc or atc0005/check-cert. In those projects the config setup attempts to retrieve a user-provided value and if not found falls back to an explicit default for each configuration setting.

While a little more work to setup initially, it makes the "getter" methods consistent and provides a stable point of reference when updating configuration setting documentation (such as discussed here).

@atc0005 atc0005 added the bug Something isn't working label Jul 12, 2020
@atc0005
Copy link
Owner Author

atc0005 commented Jul 12, 2020

See also the Configuration File section of the README where Multi-line array note is listed for one setting, but not the other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

1 participant