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

Change appendenvpath default #3281

Merged
merged 5 commits into from
Feb 2, 2019

Conversation

dmoody256
Copy link
Contributor

@dmoody256 dmoody256 commented Jan 28, 2019

Potential solution for #3276

Change the default to not touch the path.

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated master/src/CHANGES.txt directory (and read the README.txt in that directory)
  • I have updated the appropriate documentation

@mwichmann
Copy link
Collaborator

Looks like there's a test that depends on the old default behavior.

@dmoody256
Copy link
Contributor Author

I checked the related documentation in the man page, and it looks like its correct already, so that means the code before this PR was not in line with the documentation.

@bdbaddog
Copy link
Contributor

bdbaddog commented Jan 31, 2019 via email

@dmoody256
Copy link
Contributor Author

I thought the docs didn't specify the default for delete_.. parameter?

They don't say an explicit default value, but they do describe the default behavior as:

This will only add any particular path once (leaving the last one it encounters and ignoring the rest, to preserve path order)

@bdbaddog
Copy link
Contributor

bdbaddog commented Feb 1, 2019

So this is ready to merge right?
mingw failure expected at this point?

@dmoody256
Copy link
Contributor Author

dmoody256 commented Feb 2, 2019 via email

@bdbaddog bdbaddog merged commit 1e17391 into SCons:master Feb 2, 2019
@dmoody256 dmoody256 mentioned this pull request Feb 3, 2019
1 task
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.

3 participants