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

replace the old building and packaging tools with poetry #1638

Closed
wants to merge 155 commits into from

Conversation

HosamAlmoghraby
Copy link
Contributor

@HosamAlmoghraby HosamAlmoghraby commented Jan 4, 2022

  • With the changes here I am adding poetry as dependencies and packaging management tool to cookiecutter. as setuptools and wheels are not needed anymore, they were removed from the ci workflows.

  • The publishing workflow is tested by building and pushing the package to the test repository of PyPI. here is the test release https://test.pypi.org/project/cookiecutter/2.0.2/

  • The CI Tests workflows are successful on all python versions.

the setup is configured in pyproject.toml file.
setup.py file is not needed anymore.
poetry will restore the metadata from pyproject.toml file
the dev requirements are defined under [tool.poetry.dev-dependencies] section in pyproject.toml file
restore the requirements in setup.py file
Copy link
Member

@browniebroke browniebroke left a comment

Choose a reason for hiding this comment

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

Not a cookicutter maintainer, but I have some experience with Poetry, I've noted a couple of things you might find useful.

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
* run actions in self-hosted runner

* run lint ci on ubuntu-latest runner

* setup poetry-install ci job

* add tox to dev-dependencies and restrict dev-dependencies versions

* use actions/setup-python verion 2 and install dev dependencies through poetry

* remove no-dev option

* poetry install all dev dependencies

* use actions/setup-python verion 1

* try actions/setup-python verion 2

* add poetry to build job

* use poetry-core version 1.10

* remove setup.py file

* remove test_requirements.txt file

* add poetry to tox

* remove the publish part from Makefile

* use actions/setup-python verion 1

* add no-root option to poetry install

* add poetry to PATH

* try install poetry from snok actions

* restore install poetry

* install poetry

* install poetry only once

* install poetry

* use python version 3.10

* use pip in tox

* use pip in tox

* run pip within poetry

* run pre-commit within poetry

* restore poetery install in tox file

* whitelist poetry

* remove __init__ file from tests directry

* restore __init__ file to tests directry

* install dependencies in tox

* use python version 1

* use actions/setup-python verion 2

* run pytest in tox

* remove pytest from tox

* restore test_requirements file

* remove test_requirements file

* match matrix.os with windows

* check runner matrix.os if windows

* check runner matrix.os if windows

* print console matrix.os

* print console matrix.os

* use matrix.name instead of matrix.os

* remove mistuping

* add part was removed by mistake

* fix set PATH for windows os

* print GITHUB_PATH and APPDATA to console

* print GITHUB_PATH and APPDATA to console

* print GITHUB_PATH and APPDATA to console

* print env to console

* print env to console for linux machines

* print env to console

* try appsolute path to poetry in windows

* printout GITHUB_PATH file

* printout GITHUB_PATH file

* printout GITHUB_PATH file

* default shell is bash

* remove printout env

* remove pytest-plugin

* remove get full python version step

* remove poetry install from tox

* remove poetry run from tox

* add poetry run to tox

* use python verison 3.9

* use python version 3.10 and remove spaces

* rename install poetry step

* remove default shell

* restore default shell

* add step: get full python version

* tey matrix.install-args

* remove get full python version and setup cache

* revert changes of today

* try snok/install-poetry action

* add setup-python action

* continue with snok/install-poetry action

* continue with snok/install-poetry action

* continue with snok/install-poetry action

* continue with snok/install-poetry action

* continue with snok/install-poetry action

* try default shell sh

* revert default shell to bash

* continue with snok/install-poetry action

* continue with snok/install-poetry action

* continue with snok/install-poetry action

* continue with snok/install-poetry action

* continue with snok/install-poetry action

* revert snok/install-poetry action

* setup poetry action

* setup poetry action

* setup poetry action

* setup poetry action

* setup poetry action

* refactor python to python-version

* setup poetry action

* remove empty lines

* setup poetry action

* setup poetry action

* setup poetry action

* add poetry to docs workflow

* add poetry to pip-publish workflow

* run publish workflow on push branch

* finalyze the poetry work

* update maintainers and dependencies pattern

* install dependencies in tox file

* add dev dependencies to pyproject file

* run the pipeline on branch changes

* try remove skip_install from tox

* poetry install dependencies in tox testenv:lint and testenv:docs

* use pre-commit version 2

* use pre-commit version 2

* use pre-commit version 2 with python version 3.6

* poetry install dependencies in the main testenv

* non-isolated builds

* non-isolated builds

* use isolated builds

* remove the pipeline running on branch changes

* publish test release version 2.0.3
merge master to publish_with_poetry branch
Copy link
Member

@insspb insspb left a comment

Choose a reason for hiding this comment

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

I am not sure about poetry benefits, but at least errors should be fixed.

release-process.md Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@insspb
Copy link
Member

insspb commented May 29, 2022

Squash on merge.

@HosamAlmoghraby HosamAlmoghraby changed the title merge poetry replace the old building and packaging tools with poetry May 29, 2022
@jensens
Copy link
Contributor

jensens commented May 31, 2022

With 2.1.0 we follow now the official PyPA process for Github actions as defined here: https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/

As Cookiecutter defines itself as a conservative project (https://cookiecutter.readthedocs.io/en/latest/CONTRIBUTING.html#implement-features) I tend to propose not merge this PR, since following the official way is IMO the conservative way.

I personally doubt we have any advantages with Poetry here. But decision is up to the @cookiecutter/cookiecutter-sentinel core contributor team and the leads as a whole.

What do we gain using Poetry?
What problems does it solve and what are the advantages over the conservative way?

@ssbarnea
Copy link
Member

I also vote against poetry. Setuptools and PEP-517 are mature and a safe bet as of today.

@ericof
Copy link
Member

ericof commented May 31, 2022

Even though I use Poetry for my personal projects, I do not see any additional benefits here. So I'm also 👎🏼

@HosamAlmoghraby
Copy link
Contributor Author

The work here started 7 months and was waiting for a merge since January. If I start now arguing how Poetry facilitates the packaging and whether it brings advantages to the project or not and how it solves problems, then I feel something is going wrong here with the timing.

So as @jensens and @ericof are the new maintainers of Cookiecutter, here is a brief history of this PR:
Replacing the setuptools with Poetry was already discussed during the live mob programming sessions last December with @audreyfeldroy when everybody was struggling to figure out how to come up with a new release because of the lack of documentation. Back then in the Christmas holidays, I came to an agreement with @audreyfeldroy that I go deeper into how the workflow in Cookiecutter is built and I suggested leveraging the packaging to Poetry.
We agreed on a meeting on 14th January where I showed the changes I made to @pydanny and @audreyfeldroy and I also explained how Poetry brings advantages to Cookiecutter.
After that meeting, I had a few previews and I made some changes as agreed during the meeting.
Also on the same day of that meeting, I was able to publish version 2.0.3 to the test PyPI registry using the Poetry and many volunteers participated in a community testing.
Since then, we agreed several times on a meeting where we publish the new release with the new workflow but @pydanny was not able to show up because of private reasons.

@jensens
Copy link
Contributor

jensens commented Jun 1, 2022

@HosamAlmoghraby let me try to summarize:

  • the work started because of lack of knowledge how to implement a proper release process,
  • Poetry seemed to be a possible solution to overcome this
  • there were some discussion, but its not documented nor are there any facts left to built upon

Meanwhile @ericof and I implemented the official PyPA release workflow (it was like 2h work). So there is no reason anymore left from what I get to merge this. It hurts, because work and time flew into this PR, but OTHO this how evolution of software works. I feel the pain with you.

Question is - from the current state of 2.1.0: What are the benefits? What do we gain? I do not see much. One thing I like is there is no setup.py anymore. But this is possible with the official tools as well.

As said, I am conservative here. Unless I do not find a real problem solved, it is not worth to merge. Just for the sake of Poetry is not a reason.

@HosamAlmoghraby
Copy link
Contributor Author

HosamAlmoghraby commented Jun 2, 2022

  • What are the benefits? What do we gain?
    From the current state, I think, not so much (talking from a conservative point of view). The current implementation of the workflow and the other workflow proposed in this PR, both lead to package and publish Cookiecutter to PyPI.
    Poetry is an extra tool for dependency management and packaging. It manages this process out of the box, It does that like any other library used in the project to do a specific job. (click for example is used as a command-line interface in Cookiecutter!).

As long as the core team voted against the changes proposed here, I am fine with closing this PR.

@jensens jensens closed this Jun 2, 2022
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.

None yet

7 participants