-
Notifications
You must be signed in to change notification settings - Fork 6
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
Use declarative setup for SnapPy #101
base: master
Are you sure you want to change the base?
Conversation
4dcf294
to
61ab003
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks generally good.
I don't see any necessity to move all scripts from the SnapPy folder to the SnapPy/snapscripts folder. This change will destroy the environments which run scripts directly without complete installation, which is often the case.
Moving snapAddToa from the bsnap
debian package to snap-py
debian package destroys the distinction between those two packages (bsnap
: command-line tools, snap-py
: graphical user interface (heavy qt requirements)).
Too many heavy changes for a pure cleanup.
This does bring big changes, but helps reduce confusion in which package version one is running. It can be quite confusing when the import pulls in the installed package instead of the edited one. I find it more obvious to do this the pythonic way with Is the distinction between |
It is true that the scripts under utils/SnapPy consists for two reasons: Operational usage: So the operational ones could be removed from the debian snap-py package. But not enough reason to put them all into another directory. |
Rather than just moving to declarative setup.cfg, this PR requires many changes to the interna of the package. It hasn't been worked on in several month. And for a change to a declarative setup. PEP518/pyproject.toml (from 2016) should be used rather than setup.cfg. I suggest closing this PR and if still of interest, starting a new PR just replacing setup.py with pyproject.toml (for compatibility, please keep a boilerplate setup.py) |
The move to setup.cfg would greatly simplify conversion to pyproject.toml when python3.6 is phased out (pyproject.toml is not fully supported under 3.6) |
86e9659
to
c6d1b43
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR moves files, breaks in-place testing, changes filenames, breaks debian-packages and dependencies, without any benefit or reason except for transitioning to another build-system which we haven't even decided upon wanting to use. Please close silently.
No description provided.