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 poetry support; drop pipenv support; bump version #544

Merged
merged 3 commits into from
Oct 13, 2020

Conversation

dekoza
Copy link
Contributor

@dekoza dekoza commented Oct 4, 2020

No description provided.

@dekoza dekoza requested review from haxoza and tomwojcik October 4, 2020 20:05
@codecov
Copy link

codecov bot commented Oct 4, 2020

Codecov Report

Merging #544 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #544   +/-   ##
=======================================
  Coverage   98.95%   98.95%           
=======================================
  Files          19       19           
  Lines         670      670           
=======================================
  Hits          663      663           
  Misses          7        7           

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 a300b65...90bd931. Read the comment docs.

tomwojcik
tomwojcik previously approved these changes Oct 10, 2020
Copy link
Contributor

@tomwojcik tomwojcik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python3.9 is already released so it might be a good idea to add it to the tox file. Aside from that LGTM.

@tomwojcik tomwojcik dismissed their stale review October 10, 2020 15:20

found something else

@tomwojcik
Copy link
Contributor

tomwojcik commented Oct 10, 2020

I'm not experienced with poetry. I see that we now have redundant information in pyproject.toml and setup.py. Is it desired?

EDIT: I looked into it and it seems like you need to maintain setup.py if you want it to be installable via VCS. Otherwise setup.py can be removed. Is that correct? python-poetry/poetry#761 .

@dekoza
Copy link
Contributor Author

dekoza commented Oct 11, 2020

Yes, setup.py can be removed. I overlooked it.

@dekoza dekoza force-pushed the feature/poetry_support branch from d787da6 to 897a978 Compare October 11, 2020 21:57
@dekoza dekoza force-pushed the feature/poetry_support branch from 897a978 to bfc19fb Compare October 11, 2020 21:59
@dekoza dekoza merged commit 1f782ba into sunscrapers:master Oct 13, 2020
@haxoza
Copy link
Member

haxoza commented Oct 21, 2020

I was a bit afraid that once we merge it then we will receive a bunch of issues (due to lack of setup.py) from people who pinned master branch in their requirements. However after many days it seems to be not a problem at all! 🎉

@dekoza
Copy link
Contributor Author

dekoza commented Oct 22, 2020

Maybe that's because how setuptools (and pip) work right now - they respect build settings from pyproject.toml (PEP 518) and run appropriate builder (poetry-core in this case). Anyway, I'm glad too 😄

@dekoza dekoza deleted the feature/poetry_support branch October 22, 2020 09:19
haxoza added a commit that referenced this pull request Oct 22, 2020
Changes in this commit:

* Remove not used pipenv based files
* Remove dangling requirements.txt file
* Remove not used Manifest.in and setup.cfg file, which covered by
poetry
* Move flake8 config from setup.cfg to tox.ini file
* Include fuzzy translations in `make build` command
* Include *.mo files in final build

Refs #544
dekoza pushed a commit that referenced this pull request Jan 12, 2022
Changes in this commit:

* Remove not used pipenv based files
* Remove dangling requirements.txt file
* Remove not used Manifest.in and setup.cfg file, which covered by
poetry
* Move flake8 config from setup.cfg to tox.ini file
* Include fuzzy translations in `make build` command
* Include *.mo files in final build

Refs #544
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