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

Fix exclude_paths from get_playbooks_and_roles #774

Merged
merged 1 commit into from
May 11, 2020
Merged

Fix exclude_paths from get_playbooks_and_roles #774

merged 1 commit into from
May 11, 2020

Conversation

ssbarnea
Copy link
Member

@ssbarnea ssbarnea commented May 7, 2020

Fix bug which made impossible to use relative exclude paths as
these where resolved while the file tested where not.

lib/ansiblelint/utils.py Outdated Show resolved Hide resolved
@ssbarnea ssbarnea marked this pull request as ready for review May 8, 2020 08:29
@ssbarnea ssbarnea requested a review from webknjaz May 8, 2020 09:34
@cans
Copy link
Contributor

cans commented May 8, 2020

Why do you keep making competing changes ?
Similar changes were made in #749. Also your changes won't help with issue #744.
Should I close #749 ? Closed already, had not seen.

@ssbarnea
Copy link
Member Author

ssbarnea commented May 8, 2020

@cans That mention PR is huge and contains lots of unrelated changes. A PR should be kept atomic, addressing one issue, so it can easily be reviewed, merged and eventually identified if a regression occurs.

Also the code complexity is unrelated to this bug and should be addressed separately.

@webknjaz
Copy link
Member

webknjaz commented May 8, 2020

@cans I'll be glad to review the rest of the changes when submitted separately. It so happened that another PR fixed the issue and was easier to review and accept (because unrelated changes often have problems that block the whole PR even if the related changes are fine because it's hard to identify which part of the change is acceptable to merge already).

@ssbarnea I'd like to encourage you to let PR authors complete their changes if they are not urgent and received updates during the last two weeks. This way we can focus on non-conflicting changes and work in more improvement directions w/o dismissing the hard work that other folks try to contribute.

lib/ansiblelint/utils.py Outdated Show resolved Hide resolved
test/TestUtils.py Outdated Show resolved Hide resolved
test/TestUtils.py Outdated Show resolved Hide resolved
test/TestUtils.py Outdated Show resolved Hide resolved
lib/ansiblelint/utils.py Outdated Show resolved Hide resolved
Fix bug which made impossible to use relative exclude paths as
these where resolved while the file tested where not.
@ssbarnea ssbarnea requested a review from webknjaz May 9, 2020 12:10
@webknjaz webknjaz merged commit c7b714e into ansible:master May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants