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

Fix install fail when Python version >= 3.10 #61

Merged
merged 4 commits into from
Apr 6, 2022

Conversation

Merinorus
Copy link
Contributor

@Merinorus Merinorus commented Apr 5, 2022

A bash condition leads to Python 3.10 being "lower" than Python 3.7, thus the installation of fastapi-mvc would fail.

Checklist:

  • Added tests for changed code where applicable.
  • Documentation reflects the changes where applicable.
  • Updated the CHANGELOG.md file with your changes.
  • My PR is ready to review.

Resolves: #60

Description of the changes being introduced by the pull request: The python version bash condition is changed so Python version >=10 should also pass.

A bash condition leads to Python 3.10 being "lower" than Python 3.7, thus the installation of fastapi-mvc would fail.
build/install.sh Outdated Show resolved Hide resolved
@rszamszur rszamszur self-assigned this Apr 5, 2022
@rszamszur rszamszur added the bug Something isn't working label Apr 5, 2022
@Merinorus
Copy link
Contributor Author

Should I implement #62 in the same PR or should we let this good first issue to anyone that would like to contribute?

@rszamszur
Copy link
Member

@Merinorus Feel free to implement #62 as well if you'd like. Even though it's an easy issue, I don't think it should wait just because of that (at least in this case).

Also add Python 3.10 to CI (implements fastapi-mvc#62)
@Merinorus
Copy link
Contributor Author

Oops... Github actions thinks it's Python 3.1 instead of 3.10...
actions/setup-python#160 (comment)

@rszamszur
Copy link
Member

Yep, I also didn't see that coming 😆

@codecov-commenter
Copy link

codecov-commenter commented Apr 6, 2022

Codecov Report

Merging #61 (a29965a) into master (467f29b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #61   +/-   ##
=======================================
  Coverage   99.49%   99.49%           
=======================================
  Files          34       34           
  Lines         591      591           
=======================================
  Hits          588      588           
  Misses          3        3           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acdde1d...a29965a. Read the comment docs.

@rszamszur rszamszur linked an issue Apr 6, 2022 that may be closed by this pull request
1 task
@Merinorus
Copy link
Contributor Author

Merinorus commented Apr 6, 2022

Lol, I also thought it was an easy fix!
I think we need to update tlake8 to 4.0.0 at least.
PyCQA/flake8#1372

@rszamszur
Copy link
Member

Don't worry about failing CI metrics for 3.10; it seems there is a typo.

"$POETRY_HOME"/bin/poetry run flake8 --select=T --tee  --exclude fastapi_mvc/template--output-file=todo_occurence.txt --statistics --count fastapi_mvc tests
# VS
"$POETRY_HOME"/bin/poetry run flake8 --select=T --tee  --exclude fastapi_mvc/template --output-file=todo_occurence.txt --statistics --count fastapi_mvc tests

But I wonder how it didn't fail before for other py versions.

@rszamszur
Copy link
Member

And even if in fact flake8 version bump will be needed it can be done via the separate issue and PR.

If K8s integration will pass, this is good to merge.

Thanks again for the contribution!

@rszamszur rszamszur merged commit 5f452d8 into fastapi-mvc:master Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add python 3.10 in CI tests Installation fails on Python 3.10+
3 participants