-
Notifications
You must be signed in to change notification settings - Fork 24
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
Support per-repo config properly #256
Conversation
Current Aviator status
This PR was merged using Aviator.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
FlexReview SummaryBased on the code complexity and the author's expertise score, these are the suggested reviewers:
See the list of alternate reviewers in the detailed breakdown below. Detailed BreakdownAuthor’s expertise score for the modified files:
† Indicates that the file doesn't need an expert review. (?) See full breakdown of the reviewers on the Aviator webapp. |
// viper.SupportedExts. | ||
// | ||
// Note that Viper will find only one file in these directories, so if there are multiple, | ||
// only one is read. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you describe in comments which order does it detect config files in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
logrus.WithError(err).Warning("failed to determine $GIT_COMMON_DIR") | ||
} else { | ||
logrus.WithField("git_common_dir", gitCommonDir).Debug("loaded Git repo") | ||
repoConfigDir = filepath.Join(gitCommonDir, "av") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh so we were not joining av
before anywhere for repo level config?
I understand that this is a breaking change, but i guess no one would be setting .git/config.yaml
for av CLI. This is what we have in our public docs:
https://docs.aviator.co/aviator-cli/configuration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. So, I guess nobody could use per-repo config, unless they read the code.
Previously, it finds a config in some config dirs or in `.git/config.$EXT`. This changes the config loading code to do the followings: * Change the repository config path from `$GIT_DIR/config.yaml` to `$GIT_COMMON_DIR/av/config.yaml`. `$GIT_COMMON_DIR` is always (e.g. `.git` dir). `$GIT_DIR` can point to something else under certain situations. * Loads both global configs (`~/.config/av/config.yaml`) and repository config (`.git/av/config.yaml`). The repository config overrides the global config.
242bdc2
to
2319d18
Compare
Previously, it finds a config in some config dirs or in
.git/config.$EXT
. This changes the config loading code to do thefollowings:
Change the repository config path from
$GIT_DIR/config.yaml
to$GIT_COMMON_DIR/av/config.yaml
.$GIT_COMMON_DIR
is always (e.g..git
dir).$GIT_DIR
can point to something else under certainsituations.
Loads both global configs (
~/.config/av/config.yaml
) and repositoryconfig (
.git/av/config.yaml
). The repository config overrides theglobal config.