Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

BUGFIX: 627 fix include order#628

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
DanielSiepmann wants to merge2 commits intogitpython-developers:masterfromDanielSiepmann:hotfix/627-fix-include-order
Closed

BUGFIX: 627 fix include order#628

DanielSiepmann wants to merge2 commits intogitpython-developers:masterfromDanielSiepmann:hotfix/627-fix-include-order

Conversation

DanielSiepmann
Copy link

@DanielSiepmannDanielSiepmann commentedMay 14, 2017•
edited
Loading

  • Fix order of parsing configured include files.
  • More specific files should overrule included files.
  • Include files at same level where include directive happens.

Resolves:#627

* 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
Copy link
Author

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.

Copy link
Member

@ByronByron left a 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

num_read_include_files += 1
include_paths.reverse()
Copy link
Member

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.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ByronByronByron requested changes

Assignees
No one assigned
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@DanielSiepmann@Byron
[8]ページ先頭
©2009-2025 Movatter.jp