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

Add quotes to virtualenv template #6265

Merged
merged 3 commits into from Feb 9, 2020
Merged

Add quotes to virtualenv template #6265

merged 3 commits into from Feb 9, 2020

Conversation

kmaragon
Copy link
Contributor

@kmaragon kmaragon commented Dec 20, 2019

Changelog: BugFix: Add quotes to virtualenv scripts, so they don't crash in pure sh shells.
Docs: Omit

Close #6264 with required quotes

  • Refer to the issue that supports this Pull Request.
  • N/A If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • N/A I've followed the PEP8 style guides for Python code.
  • N/A 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.

Fix #6264 with required quotes
@claassistantio
Copy link

claassistantio commented Dec 20, 2019

CLA assistant check
All committers have signed the CLA.

@jgsogo
Copy link
Contributor

jgsogo commented Dec 20, 2019

Hi, @kmaragon ! Thanks a lot for taking care of #6264. I'll ask you a couple of things before merging this PR:

  • this PR should be merged into branch release/1.21.1, we want to fix it in the next minor, so please, change the target branch (do not include other changesets from branch develop)
  • tests are failing due to an extra blank. That is something marked as a FIXME so it is ok to add the extra blank and ensure that tests pass.

On the other hand, I'll propose a PR to develop changing our tests to more vanilla sh trying to reproduce the error reported originally, once those tests are ready they should show the error and this same changeset should fix them (I'll cherry-pick your changeset). Note.- If you want to add those tests here too it would be perfect, of course.

(Let us know if you need any help with the changes requested)

Thanks a lot!

@jgsogo jgsogo added this to the 1.21.1 milestone Dec 20, 2019
@jgsogo jgsogo self-assigned this Dec 20, 2019
@jgsogo jgsogo changed the base branch from develop to release/1.21.1 December 30, 2019 17:23
@jgsogo jgsogo changed the base branch from release/1.21.1 to develop January 7, 2020 19:04
@jgsogo jgsogo modified the milestones: 1.21.1, 1.22 Jan 7, 2020
@jgsogo jgsogo removed their assignment Jan 7, 2020
@memsharded memsharded removed this from the 1.22 milestone Jan 29, 2020
@jgsogo jgsogo removed their assignment Feb 3, 2020
@memsharded memsharded changed the base branch from develop to release/1.22.1 February 8, 2020 23:07
@memsharded memsharded added this to the 1.22.1 milestone Feb 8, 2020
@memsharded memsharded merged commit e564104 into conan-io:release/1.22.1 Feb 9, 2020
@memsharded memsharded added this to Closed in Conan virtualenvs via automation Feb 9, 2020
jgsogo added a commit to jgsogo/conan that referenced this pull request Feb 10, 2020
jgsogo added a commit that referenced this pull request Feb 10, 2020
memsharded added a commit to memsharded/conan that referenced this pull request Feb 11, 2020
* Add quotes to virtualenv template

Fix conan-io#6264 with required quotes

* fixing tests

Co-authored-by: James <james@conan.io>
czoido pushed a commit that referenced this pull request Feb 14, 2020
* Add quotes to virtualenv template (#6265)

* Add quotes to virtualenv template

Fix #6264 with required quotes

* fixing tests

Co-authored-by: James <james@conan.io>

* remove code without effect in test

Co-authored-by: Keef Aragon <keef.aragon@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

[bug] New virtualrunenv template: $: bad variable name
4 participants