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

Remove double application of env vars from profile #4380

Merged
merged 10 commits into from Jan 28, 2019

Conversation

Projects
None yet
4 participants
@danimtb
Copy link
Member

commented Jan 24, 2019

Changelog: BugFix: Prepend environment variables are applied twice in conanfile
Docs: omit

  • Refer to the issue that supports this Pull Request: closes #4385
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

@ghost ghost assigned danimtb Jan 24, 2019

@ghost ghost added the stage: review label Jan 24, 2019

@unittest.skipUnless(platform.system() == "Windows", "Test both .sh and .bat files")
class VirtualenvGeneratorTest(unittest.TestCase):

def prepend_values_test(self):

This comment has been minimized.

Copy link
@danimtb

danimtb Jan 25, 2019

Author Member

This test checks that is not the fault of the virtualenv generator

@danimtb danimtb assigned memsharded and unassigned danimtb Jan 25, 2019

@danimtb danimtb requested a review from lasote Jan 25, 2019

@danimtb

This comment has been minimized.

Copy link
Member Author

commented Jan 25, 2019

Tried to make unit tests for this but I ended up with a lot of objects with values pre loaded just to test the loader methods... 🙄 So maybe it is better to add just a functional test for conanfile.txt and check the environment prepended values with the virtualenv generator output

@danimtb danimtb changed the title WIP: Remove double application of env vars from profile Remove double application of env vars from profile Jan 25, 2019

@danimtb danimtb added the type: bug label Jan 25, 2019

@memsharded memsharded added this to the 1.12 milestone Jan 25, 2019

@ghost ghost assigned danimtb Jan 28, 2019

@danimtb danimtb removed their assignment Jan 28, 2019

@ghost ghost assigned danimtb Jan 28, 2019

@danimtb danimtb removed their assignment Jan 28, 2019

@ghost ghost assigned danimtb Jan 28, 2019

@lasote

lasote approved these changes Jan 28, 2019

@memsharded memsharded merged commit 27b92f0 into conan-io:develop Jan 28, 2019

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details

@ghost ghost removed the stage: review label Jan 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.