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

[Resolves #978] Only install typing on Python<3.5 #979

Merged
merged 5 commits into from
Apr 15, 2021

Conversation

gabriellesc
Copy link
Contributor

This PR constrains the installation of typing to Python<3.5, since it is included in the standard library as of 3.5 and causes issues if installed alongside the standard library version.

PR Checklist

  • Wrote a good commit message & description [see guide below].
  • Commit message starts with [Resolve #issue-number].
  • Added/Updated unit tests.
  • Added/Updated integration tests (if applicable).
  • All unit tests (make test) are passing.
  • Used the same coding conventions as the rest of the project.
  • The new code passes flake8 (make lint) checks.
  • The PR relates to only one subject with a clear title.
    and description in grammatically correct, complete sentences.

Approver/Reviewer Checklist

  • Before merge squash related commits.

Other Information

Guide to writing a good commit

@zaro0508
Copy link
Contributor

zaro0508 commented Apr 2, 2021

maybe should just remove typing altogether since sceptre currently only supports python 3.6 and 3.7? https://github.com/Sceptre/sceptre/blob/master/tox.ini#L2

@chrono
Copy link

chrono commented Apr 6, 2021

Anything else needed to merge this?

@ykhalyavin
Copy link
Contributor

maybe should just remove typing altogether since sceptre currently only supports python 3.6 and 3.7? https://github.com/Sceptre/sceptre/blob/master/tox.ini#L2

Makes sense

@tarkatronic
Copy link
Member

@gabriellesc Do you have time to update this and fully remove typing?

@zaro0508 If Gabrielle is not able to get this updated, would you be willing to merge another PR to take care of this? And would there be a possibility of a bugfix release to include this update?

This has become a hard blocker for me, as I cannot concurrently install sceptre and black into my projects.

@zaro0508
Copy link
Contributor

zaro0508 commented Apr 9, 2021

@zaro0508 If Gabrielle is not able to get this updated, would you be willing to merge another PR to take care of this? And would there be a possibility of a bugfix release to include this update?

yes @tarkatronic, willing to merge another PR for this however we are having difficulties with making a release at this moment. we are having discussions and trying to resolve in the #sceptre slack channel.

@craighurley
Copy link
Contributor

Hi @tarkatronic , to work around module conflicts I run sceptre in a container. If this is something that would work for you in this case, have a look at https://github.com/craighurley/docker-sceptre

@gabriellesc
Copy link
Contributor Author

Fully removed the typing dependency as suggested! Let me know if there is anything I missed.

@zaro0508 zaro0508 merged commit 5ee9cde into Sceptre:master Apr 15, 2021
@gabriellesc gabriellesc deleted the patch-1 branch April 15, 2021 17:19
alex-harvey-z3q pushed a commit to fsa-streamotion/sceptre that referenced this pull request May 14, 2021
This PR constrains the installation of `typing` to Python<3.5, since it is included in the standard library as of 3.5 and causes issues if installed alongside the standard library version.
mrowlingfox pushed a commit to fsa-streamotion/sceptre that referenced this pull request May 19, 2021
This PR constrains the installation of `typing` to Python<3.5, since it is included in the standard library as of 3.5 and causes issues if installed alongside the standard library version.
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.

7 participants