Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork966
BUGFIX: 627 fix include order#628
BUGFIX: 627 fix include order#628DanielSiepmann wants to merge 2 commits intogitpython-developers:masterfromDanielSiepmann:hotfix/627-fix-include-order
Conversation
* Add include files to the beginning to parse them right after the file containing the include directive.* This way more specific config files will overrule the configurations.Resolves:#627
DanielSiepmann commentedMay 14, 2017
There might be a pythonic way of solving this, but as I'm a beginner im happy to provide at least a fix for the issue. |
Byron left a comment
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.
First of all, sorry for letting this PR wait for so long, despite me being super happy about every contribution!
So thanks a lot for taking the time to make it!
Besides that simple improvement in readability, do you think you can figure out howto adjust this test-case to assert for the change this PR is making?
You should be able run the config tests vianosetests git/test/test_config.py from the root of this repository.
If adjusting the test doesn't work for you, after all it's not particularly easy to understand and quite complex/convoluted, please let me know and we find another way.
Thanks and cheers
| files_to_read.append(include_path) | ||
| include_paths.append(include_path) | ||
| num_read_include_files += 1 | ||
| include_paths.reverse() |
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.
I think this block should be rewritten like this:
files_to_read=include_paths+files_to_read
This makes clearer that included files should be read and merged first.
Uh oh!
There was an error while loading.Please reload this page.
Resolves:#627