Command Line Options for Interacting with the Configuration Merging Process #1748
Conversation
|
Since I guess I cannot request reviews directly, this is a callout to you, @marcelofabri. |
Generated by 🚫 Danger |
|
Any idea why only Linux would fail to accurately compare two |
Codecov Report
@@ Coverage Diff @@
## master #1748 +/- ##
==========================================
- Coverage 87.75% 87.69% -0.07%
==========================================
Files 218 218
Lines 10704 10781 +77
==========================================
+ Hits 9393 9454 +61
- Misses 1311 1327 +16
Continue to review full report at Codecov.
|
|
The code coverage report is misleading. The two affected files in |
|
The single Linux failure only occurred in 3/4 times with no changes to the relevant code. (See the CI logs for the four separate commits above). The test is older than this pull request. It is an It reminds me of chatter a while ago about a similar issue on Linux to do with caches not matching because set/dictionary iteration order is not deterministic. Since sets are being used in Adding a sort step would slow things down, but I notice (lhs.rules == rhs.rules)↓ (lhs.rules.sorted(by: { String(describing: $0) < String(describing: $1) })
== rhs.rules.sorted(by: { String(describing: $0) < String(describing: $1) }))Thoughts? Or do you want to keep that issue distinct from this pull request? |
|
I will push the above fix for the Linux tests. (If nothing else, for the nice green checkmark.) Since maintainers have write access to the branch, if you decide a different solution would be better, you can go ahead make the change. |
|
Everything is ready now. |
|
Merge conflicts have accumulated since the original proposal. Since this pull request has never received any reviews or comments, the work of updating it does not seem worth it. I am therefore closing it. (I will wait a week before I actually delete my branch in case there really are others out there who want something like this.) |
|
Well, my company need this too, but guys in charge don't think it's a good feature. Have to find another solution |
|
@Aranoledur, you are welcome to pull my changes to your own fork if you want to maintain one. Just do it sooner rather than later, as I will not likely keep my fork around forever. |
(From #1693, expanded from @jpsim’s suggestions in #1323.)
Adds three command line options:
--config-defaults: the path of an external configuration file to use as the root of the merge tree.--config-overrides: the path of an external configuration file to append to the end of each branch of the merge tree--ignore-nested-configs: ignores nested configuration files(The combination of
--config-defaultsand--ignore-nested-configscan be used to make--config-defaultsstomp anything present in the project, even the root directory.)Commit breakdown:
The interaction with the merge logic is isolated in the second commit. (The first commit simply adds the options to the interface and gets them to
Configuration.)Other Notes:
I left
--configalone for now, but I think this pull request would make it obsolete. (In the wake of #1687 it looks like it merges itself with anything it finds, all the way back to the system root, which I doubt anyone expects.)The read‐me is out of date. It still says nested files trump their parents. I left it alone in case you want it in sync with the last stable release and not master.
In the process of writing this, I discovered that the existing merge process went backwards. A tree like this...
A
↳ B
↳ C
↳ D
...was being merged as A is overridden by D, which is overridden by C, which is overridden by B.
The refactors in this pull request eliminate the problem, but if this pull request is not accepted, that bug will have to be dealt with elsewhere.
Maintainers have been given direct write access.
(@Aranoledur, you were tracking the related issue, so you might want to look at this.)