for_lane, for_platform blocks in configurations #7859
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 tasks
mfurtak
changed the title
mfurtak
commented
Jan 14, 2017
| # Setting this variable to be true allows a one-time overwrite of a particular | ||
| # configuration value. This is used by the override block methods to allow | ||
| # giving a more specific config value for a particular circumstance | ||
| @allow_overwite = false |
There was a problem hiding this comment.
Whoops, this should be removed. Is leftover from my first try/approach 😦
KrauseFx
reviewed
Jan 15, 2017
| # lane_name - Symbol representing a lane name. | ||
| # block - Block to execute to override configuration values. | ||
| # | ||
| # Discussion If received lane name does not match the lane name available as environment variable, no changes will |
There was a problem hiding this comment.
Hm, this was from the original PR. I can make the comments look more like our usual documentation. 👍
KrauseFx
reviewed
Jan 15, 2017
| # configuration for applying values, so that values can be overridden | ||
| # (once) again. Those values are then merged into the surrounding | ||
| # configuration as the block completes | ||
| def with_a_clean_config_merged_when_complete |
KrauseFx
reviewed
Jan 15, 2017
| @@ -119,6 +119,60 @@ | |||
| end | |||
| end) | |||
| end | |||
|
|
|||
| describe "for_lane and for_platform support" do | |||
There was a problem hiding this comment.
Perfect, just new tests, without modifying any of the existing ones 👍
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A continuation of work on #6549
This reconciles the new configuration blocks (
for_lane,for_platform) and their desire to be able to override configuration values, with the existing rules about "first assignment wins" for configurations.It does this by having the new configuration blocks create a fresh configuration context every time you enter one. This way, each block gets a fresh application of the "first assignment wins" rule. As each block exits, any values set during the block are merged with any configuration context that was present at the time the block started. Thus, we can override values in these blocks, but only once, and still build a single comprehensive configuration!
This is supported with new
push_values!andpop_values!methods onConfigurationIf we like the solution, we can always add documentation as a follow-up.
/cc @maschall