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

Target variables #417

Merged
merged 18 commits into from Jan 23, 2019
Merged

Target variables #417

merged 18 commits into from Jan 23, 2019

Conversation

theory
Copy link
Collaborator

@theory theory commented Dec 30, 2018

Add support for core, engine, and target variable configuration, as requested in #275. The simplest way to use them is via the --set option on the init, engine, and target commands. These allow the configuration of database client variables for specific engines and targets, as well as defaults that apply to all change execution commands (deploy, revert, verify, rebase, and checkout). The commands merge the variables from each level in this priority order:

  • --set-deploy and --set-revert options on rebase and checkout.
  • --set option
  • target.$target.variables
  • engine.$engine.variables
  • deploy.variables, revert.variables, and verify.variables
  • core.variables

See sqitch-configuration for general documentation of of the hierarchy for merging variables and the documentation for each command for specifics.

@theory theory added the feature label Dec 30, 2018
@theory theory added this to the v1.0.0 milestone Dec 30, 2018
@theory theory self-assigned this Dec 30, 2018
@coveralls
Copy link

coveralls commented Dec 30, 2018

Pull Request Test Coverage Report for Build 341

  • 88 of 88 (100.0%) changed or added relevant lines in 11 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 93.785%

Totals Coverage Status
Change from base Build 1146: 0.1%
Covered Lines: 4195
Relevant Lines: 4473

💛 - Coveralls

Copy link
Contributor

@ssoriche ssoriche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read through all the code and documentation. I really like this feature. Merge conflicts need to be cleaned up though.

@theory
Copy link
Collaborator Author

theory commented Jan 20, 2019

Thanks @ssoriche. I'll be rebasing before I merge.

The new `--set`/`-s` option for the `init`, `engine`, and `target` commands
configures change script variables at the core, engine, and target levels,
respectively. The get written to their own config section, but the new
`variables` accessor on the Target object will merge them all.

So far, only the target command is fully updated and tested.
And show no variables.
And update `verify` to also read `deploy` variables, just like `revert` does.

Variables merge from most to least specific, in general as follows:

1. `--set` options
2. `target.$target.variables`
3. `engine.$engine.variables`
4. `$command.variables`
5. `core.variables`

They don't work that way yet, but will soon.

Other changes:

* Fix a couple typos.
* Eliminate duplicate calls to `$self->sqitch->config`
* Document that `rework` and `checkout` also have `--set` options.
Because of the prioritization of command variables, we have to remove them
here so that the commands can specify the proper hierarchies.
I forgot it was a subargument to catch, not a block. Thanks to @stefansbv
for the pointer.
@theory theory merged commit 6b6469c into master Jan 23, 2019
@theory theory deleted the target-variables branch February 27, 2019 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants