twisted / ldaptor

LDAP server, client and utilities, using Twisted Python
MIT License
160 stars 53 forks source link

use version attr: to extract version with declarative setuptools #212

Closed graingert closed 4 years ago

graingert commented 4 years ago

Contributor Checklist:

graingert commented 4 years ago

@adiroiban we can now delete the setup.py (as long as we don't need to support pip install -e ldaptor)

codecov[bot] commented 4 years ago

Codecov Report

Merging #212 into master will decrease coverage by 0.00%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #212      +/-   ##
==========================================
- Coverage   83.30%   83.29%   -0.01%     
==========================================
  Files          87       87              
  Lines       11569    11569              
  Branches     1184     1184              
==========================================
- Hits         9637     9636       -1     
  Misses       1815     1815              
- Partials      117      118       +1     
Impacted Files Coverage Δ
ldaptor/protocols/ldap/ldapsyntax.py 94.24% <0.00%> (-0.22%) :arrow_down:

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 2febc41...2c33761. Read the comment docs.

adiroiban commented 4 years ago

@graingert if pip install -e is no longer supported, does it mean that when you make a change and you want to run a test you need to recreate a wheel/sdist and install ?

I am always using pip install -e . for developmet

graingert commented 4 years ago

Yeah, no more -e I tend to use tox py38-test-twlatest

if we switch to flit or peotry they have a symlink mode which is a bit nicer than pth based

adiroiban commented 4 years ago

tox is slow ... it takes 40seconds for a second run, after all deps were already installed

In my view tox is for CI and automated test execution and not for TDD.

It takes 1 second to run all 672 tests with trial in dev mode. This is TDD friendly :)

adiroiban commented 4 years ago

sometimes I am also using tox to setup the virtualenv with

tox -e py38-test-dev --develop
 . .tox/py38-test-dev/bin/activate
trial ldaptor
graingert commented 4 years ago

@adiroiban https://discuss.python.org/t/next-steps-for-editable-develop-proof-of-concept/4118/28

adiroiban commented 4 years ago

@graingert ... sorry, but I don't undestand the root problem.

Why we can't have ldaptor install in dev mode and why we need extra hacks?

ldaptor has no binary module / C extension so at develop time there is no need to recompile any extension.

But, do as you think is best. My comment is just to make sure contributing to ldaptor is not a slow and painful process.

I think that pip install -e . is a common tool and people are using it without having to read many extra pages of documentation and information on how to contribute to a project

graingert commented 4 years ago

@adiroiban there's currently a movement to decouple package building from package installing:

adiroiban commented 4 years ago

Many thanks for the detailed explanation.

my undetstanding is that pip install -e . is not yet deprecated and currently there is no standardized alternative. So python setup.py develop via pip install -e . is still the recommended way to install in dev mode.

So I think that untill there is a replacement for python setup.py develop we can keep the setup.py. This is a small file and I don't see any issue with continue keeping it.

Thanks again for the update from this PR :)

graingert commented 4 years ago

The setup.py is indeed a small file, but it exposes the legacy setuptools interface to consumers of the sdist