Commit Graph

9 Commits

Author SHA1 Message Date
risk danger olson
2e213cf9c4 config: remove env mutation funcs, no longer needed! 2017-01-10 13:25:03 -07:00
risk danger olson
1715e97f41 add export comments 2016-11-10 11:24:45 -07:00
risk danger olson
b694d64f12 make set() and del() private 2016-11-10 11:16:46 -07:00
risk danger olson
0e679c7d98 add Set(), remove .gitConfig 2016-11-09 17:46:52 -07:00
risk danger olson
e2ca1e5533 add Environment Del(key string) 2016-11-09 17:37:14 -07:00
risk danger olson
9c46443baf remove AllGitConfig() 2016-11-09 17:33:43 -07:00
risk danger olson
9c727d210c change (Environment) Get()'s signature to include ok bool 2016-08-05 15:59:57 -06:00
Taylor Blau
18cb9257f9 config: demote Fetcher, introduce Environment
Previously, to fetch data out of the `*config.Configuration` type, a reference
to a `Fetcher` was used, a-la:

```
cfg.Env.Get(...)
```

This is quite convenient, however, it forces the LFS client to implement
several methods more than once. Consider the interface:

```
type Fetcher interface {
        Get(key string) (val string)

        Bool(key string, def bool) (val bool)
        // et. al.
}
```

In order to return typed information from a configuration instance, _each_
`Fetcher` must implement its own `N` methods for `Int`, `Bool`, etc.

To remedy this, the `Environment` type was introduced. It instead _has_ a
`Fetcher`, and defines its own type conversions, like so:

```
type Environment struct {
        f Fetcher
}

func (e *Environment) Bool(key string, def bool) (val bool) { }
func (e *Environment) Int(key string, def int)   (val int) { }

// et. al.
```

Now, the `config.Configuration` type holds a reference to an `Environment`, and
all type conversion methods are defined only once, saving time, and enforcing
consistency across multiple sources.
2016-08-04 14:28:19 -06:00
Taylor Blau
e32fabe37f config: introduce Fetcher type
As per my comments in
https://github.com/github/git-lfs/pull/1390#issuecomment-236068550, I'd like
the `*config.Config` type to be focused solely on data fetching, not having to
know about the domain of each of its callers. The `Fetcher` type is the first
step in that direction.

`Fetcher` is an interface designed to be implemented in three ways, concrete
types that wrap:
  1) Interaction with the `.gitconfig`.
  2) Calls to `os.Getenv`.
  3) A `map[string]string` for testing.

In this initial implementation, the `Fetcher` is responsible for type
conversion, providing methods like `Fetcher.Bool` and (eventually)
`Fetcher.Int`.

The above is undesirable for a number of reasons:
  1) It forces implementers of the `Fetcher` type to be responsible not only
    for data-fetching, but also data conversion.
  2) At the time of writing, data conversion is uniform throughout the LFS
    codebase. This encourages code smells by way of duplication in that all
    types will have to implement the same sort of type conversions with the
    same sort of code. The alternative is to have static helper methods, or
    weird abstraction trees, but this is similarly undesirable.

In subsequent PRs, I plan to demote the Fetcher type to a function:

```
type FetchFn func(key string) (val string)
```

and introduce an `Environment` type which *has* a `FetchFn`. In doing so, I'll
delegate the responsibility of implementing the `Bool` and `Int` (etc) methods
to the concrete `Enviornment` type, and only implement it once.
2016-08-03 16:06:25 -06:00