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

Remove setup.py and tidy build scripts #1443

Merged
merged 10 commits into from
Sep 5, 2022
Merged

Remove setup.py and tidy build scripts #1443

merged 10 commits into from
Sep 5, 2022

Conversation

adehad
Copy link
Contributor

@adehad adehad commented Aug 1, 2022

Change Summary

  1. Remove setup.py (would have kept it if we were using pyproject.toml, per the guidance from setuptools docs to preserve editable install behaviour across older pip's)
  2. Move tool config to pyproject.toml where possible, only flake8 remains in setup.cfg
  3. Remove other old files incl. Makefile and other tools configs not used - can be added again if appropriate
  4. For whatever reason had to update the logic around keyring password. The previous output captured used to be No password provided\n assert ''\n, but somehow became No password provided\n. I've moved this to a simple ValueError as assertion's aren't really a great way to surface these kinds of problems. (e.g. if someone runs with -O for whatever reason)
  5. Consolidated project options in setup.cfg removing duplicates that made their way to pyproject.toml ( and the removed setup.py)

Testing the git archive can be performed using:

pip install git+https://github.com/pycontribs/jira.git@feature/pyproject
python -c "import jira; print(jira.__version__)"

@adehad adehad requested a review from ssbarnea August 1, 2022 22:47
@ssbarnea
Copy link
Member

ssbarnea commented Aug 2, 2022

@adehad That is one dangerous move as the full-pyproject support is very new in pip, likely will cause problems for many people.

@ssbarnea
Copy link
Member

ssbarnea commented Aug 2, 2022

On the other hand, I do fully support removal of setup.py.

@adehad
Copy link
Contributor Author

adehad commented Aug 2, 2022

@adehad That is one dangerous move as the full-pyproject support is very new in pip, likely will cause problems for many people.

My understanding is pretty limited, but I would have thought it wouldn't be a problem to most users as they would use the pypi wheels? Would be great to know a bit about what kinds of failures we can expect to see from your experience @ssbarnea

I know asottile also has strong opinions on this topic and prefers not even having the presence of the pyproject file as pip completely changes its behaviour when it sees it....

@ssbarnea
Copy link
Member

ssbarnea commented Aug 2, 2022

Hehe... most of his opinions are strong. I will try to give a hand later this week, now I am quite busy dealing with too many tasks in parallel.

For the moment I would recommend matching what we do on https://github.com/ansible/ansible-lint/blob/main/tox.ini -- where we removed setup.py, but did not fully migrate to pyproject.toml, making full use of setuptools-scm.

@adehad adehad force-pushed the feature/pyproject branch from 9657db6 to 9726059 Compare August 6, 2022 14:47
Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

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

Do NOT move to pyproject.toml, editable installs still do not work. There is a setuptools PR that is open for fixing them.

@adehad adehad force-pushed the feature/pyproject branch from 9726059 to 5a5e183 Compare August 7, 2022 11:11
@adehad adehad changed the title move to pyproject.toml remove setup.py and tidy build scripts Aug 7, 2022
@adehad adehad temporarily deployed to cloud August 7, 2022 12:03 Inactive
@adehad adehad closed this Aug 7, 2022
@adehad adehad reopened this Aug 7, 2022
@adehad
Copy link
Contributor Author

adehad commented Aug 7, 2022

re-running to check if codecov will fix itsef

@adehad adehad temporarily deployed to cloud August 7, 2022 12:32 Inactive
@adehad adehad force-pushed the feature/pyproject branch from 9e05bdb to 59842e9 Compare August 7, 2022 13:51
@adehad adehad temporarily deployed to cloud August 7, 2022 14:04 Inactive
@adehad adehad marked this pull request as draft August 7, 2022 19:09
@adehad adehad marked this pull request as ready for review August 7, 2022 19:29
@adehad adehad temporarily deployed to cloud August 7, 2022 19:32 Inactive
@adehad adehad requested a review from ssbarnea August 7, 2022 21:01
@adehad
Copy link
Contributor Author

adehad commented Aug 24, 2022

@ssbarnea would be great if you have a chance for a second look on this

@adehad adehad force-pushed the feature/pyproject branch from c9a9bcd to be3d8af Compare August 24, 2022 21:10
@adehad adehad temporarily deployed to cloud August 24, 2022 21:26 Inactive
@adehad adehad temporarily deployed to cloud September 4, 2022 15:29 Inactive
@ssbarnea ssbarnea changed the title remove setup.py and tidy build scripts Remove setup.py and tidy build scripts Sep 5, 2022
@ssbarnea ssbarnea merged commit 7846ac3 into main Sep 5, 2022
@ssbarnea ssbarnea deleted the feature/pyproject branch September 5, 2022 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants