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

Second attempt to fix prepending of root_dir to paths #38804

Merged
merged 6 commits into from Feb 2, 2017

Conversation

alexbleotu
Copy link
Contributor

@alexbleotu alexbleotu commented Jan 18, 2017

What does this PR do?

Second attempt to fix prepending of root_dir to paths

Fixes: #38663

Previous Behavior

Prepended the root_dir override to the default one

New Behavior

Prepend the root_dir override only

Tests written?

No

Please review Salt's Contributing Guide for best practices.

@alexbleotu
Copy link
Contributor Author

@cachedout there are several integration tests failing, but I'm not familiar with the code and don't understand the failures. Can you have a look?

@cachedout
Copy link
Contributor

Go Go Jenkins!

1 similar comment
@cachedout
Copy link
Contributor

Go Go Jenkins!

@alexbleotu
Copy link
Contributor Author

@cachedout Did you have time to look into the failed test? I can't reproduce the failure so my hands are tied

@alexbleotu
Copy link
Contributor Author

@cro This is the pull request to fix the root_dir override. It works in my environment, but it fails an integration test here

@cro
Copy link
Contributor

cro commented Jan 31, 2017

@alexbleotu I will take a look.

@cro
Copy link
Contributor

cro commented Feb 1, 2017

@alexbleotu I sent a PR with a couple cleanups to your fork. If you agree & merge, then re-push here we'll get the tests to run again. It looks like the test failures are unrelated but one set of them already got cleaned up by Jenkins so I could not look at it.

Remove extra if statements (rstrip will check for the presence anyway).
@alexbleotu
Copy link
Contributor Author

@cro done, thanks for the patch

@alexbleotu
Copy link
Contributor Author

No pylint violations in the jenkins job?

@cro cro merged commit cd8077a into saltstack:2016.3 Feb 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants