-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Makefile updates #5159
Makefile updates #5159
Conversation
wiredfool
commented
Dec 30, 2020
- Set the default goal to something unsurprising.
- Remove obsolete target (co)
- Add the lint target.
Makefile
Outdated
|
||
.PHONY: lint | ||
lint: | ||
tox --help > /dev/null || pip install tox |
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.
We use pip like this elsewhere in the Makefile:
tox --help > /dev/null || pip install tox | |
tox --help > /dev/null || python3 -m pip install tox |
Let's also put the make commands in roughly alphabetical order to make them easier to find, and add a help entry for lint:
$ make help
Welcome to Pillow development. Please use `make <target>` where <target> is one of
clean remove build products
coverage run coverage test (in progress)
doc make html docs
docserve run an http server on the docs directory
html to make standalone HTML files
inplace make inplace extension
install make and install
install-coverage make and install with C coverage
install-req install documentation and test dependencies
install-venv install in virtualenv
release-test run code and package tests before release
test run tests on installed pillow
upload build and upload sdists to PyPI
upload-test build and upload sdists to test.pythonpackages.com
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.
install-venv:
virtualenv .
bin/pip install -r requirements.txt
It's 1:1 for python3 -m pip install
vs pip install
, though to be fair, I think this is an artifact of the original usage, and I'd argue against both this and the inplace target on a general objection to mucking up the source tree. (though, I do understand that it's potentially used for some coverage or automated thing. )
Do we have a handle on what people are actually using from this makefile?
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.
Other than the release checklist commands (release-test
, sdist
) I mainly only use doc
and clean
.
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.
https://snarky.ca/why-you-should-use-python-m-pip/ is the reason for python3 -m pip
, mostly important for the docs as we often get bug reports caused by installing for a different interpreter.
Similarly, I have earlier had problems with python
in by the Makefile pointing to Python 2, not 3.
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.
Also, I'm not a fan of sorting the rules alphabetically, I'd rather group them by purpose.
(especially as there are some new ones coming in that are 1 public, and several dependent ones)
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.
isort could be handy for lint-fix too
Yeah, anything that just fixes it is good. |
Added isort to lint-fix
Add #5159 to the release notes