-
-
Notifications
You must be signed in to change notification settings - Fork 529
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
Add/update release and contributor howtos and adapt to how pytest does stuff #636
Conversation
* use new towncrier functionality render different formats * fix header levels
Codecov Report
@@ Coverage Diff @@
## master #636 +/- ##
==========================================
- Coverage 93.95% 92.63% -1.32%
==========================================
Files 11 11
Lines 2364 2364
==========================================
- Hits 2221 2190 -31
- Misses 143 174 +31
Continue to review full report at Codecov.
|
pyproject.toml
Outdated
[[tool.towncrier.format]] | ||
kind = "pull request" | ||
patterns = ["pr(\\d)", "pull-request(\\d)"] | ||
format = "`#{id} <https://github.com/tox-dev/tox/pull/{id}>`_" |
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.
FYI GitHub doesn't care if you use /pull/
or /issue/
on the URL as long as the number is correct.
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.
Really? Can't they overlap? Why did nobody tell me earlier? All this programming for nothing? I am disappoint :(
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.
oh dear ... anyway. I had a blissfull afternoon exploring towncrier and all, so who cares :)
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.
I think it's a complete UX fail then that they even have /issues and pull/ - they should just have entity/ or sometting and don't redirect from issue to pull when you ref a pull from an issue url.
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.
Yep, it is weird 😁
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.
FYI, although it is a redirect, it's a permanent redirect; so if you leave it as it is the sphinx link checker would complain about it.
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.
Thanks @gaborbernat - I will keep that in mind when finishing up today.
I consider this as done as possible now and will make sure to iron out any kinks over time. |
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.
mostly nitpicks :)
tox.ini
Outdated
@@ -39,6 +39,8 @@ description = run static analysis and style check using flake8 | |||
commands = python -m flake8 --show-source tox setup.py {posargs} | |||
python -m flake8 --show-source doc tests {posargs} | |||
pre-commit run --all-files | |||
echo "hint: run {envdir}/bint/pre-commit install" to add checks as pre-commit hook |
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.
s/bint/bin/g
You could also just run this directly: pre-commit install --overwrite
: here's an example
|
||
[testenv:style] | ||
[testenv:fix-lint] |
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.
I expect a change to appveyor.yml
(where this gets currently run) as well
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.
heh in fact this is a sneaky way to skip the style checks: https://ci.appveyor.com/project/RonnyPfannschmidt/tox/build/1.0.444/job/mfdxp0fvjcy67g94
commands = sphinx-build -d .tox/docs_doctree doc .tox/docs_out --color -W -bhtml | ||
sphinx-build -d .tox/docs_doctree doc .tox/docs_out --color -W -blinkcheck | ||
commands = sphinx-build -d {toxworkdir}/docs_doctree doc {toxworkdir}/docs_out --color -W -bhtml | ||
sphinx-build -d {toxworkdir}/docs_doctree doc {toxworkdir}/docs_out --color -W -blinkcheck |
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.
nothing worth changing I think, but I find the linkcheck really slow and really annoying when attempting to iterate on the docs -- I usually end up commenting this line out to iterate.
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.
maybe add the linkchecker as a separate env then? 👍
tasks/pre-process-changelog.py
Outdated
|
||
|
||
def main(): | ||
manipulate_the_news() |
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.
worth having main()
as just a trivial wrapper here? (personal opinion, feel free to ignore!) -- a simpler way to write this is main = manipulate_the_news
;)
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.
I can see how this can be taken in two directions: my personal opinion would be to not have this trivial wrapper and just rename the manipulate_the_news
function -> main
. But of course feel free to ignore this comment :)
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.
I would also say, just alias it
doc/conf.py
Outdated
import sys | ||
from datetime import date | ||
|
||
from pkg_resources import get_distribution | ||
|
||
sys.path.insert(0, os.path.dirname(__file__)) | ||
here = os.path.dirname(__file__) | ||
sys.path.insert(0, here) |
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.
hmm, any reason these lines changed?
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.
probably to make it more verbose 👍
HOWTORELEASE.rst
Outdated
|
||
* tox has no long lived branches. | ||
|
||
* Pull requests get integrated into master by members of the project when they feel confident that this could be part of the next release. Small fix ups might be done right after merge instead of disscussing back and forth to get minor problems fixed. to keep the workflow simple. |
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.
"... problems fixed, to keep the workflow simple."
* Release rights for https://pypi.org/project/tox/ | ||
* [optional] An account on https://devpi.net to upload the package under test | ||
* A system with either tox + `bash <https://www.gnu.org/software/bash/>`_ or `vagrant <https://github.com/tox-dev/tox/blob/master/Vagrantfile>`_ (which contains tox + bash) | ||
* Accountability: if you cut a release that breaks the CI builds of projects using tox, you are expected to fix this within a reasonable time frame (hours/days - not weeks/months) - if you don't feel quite capable of doing this yet, partner up with a more experienced member of the team and make sure they got your back if things break. |
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.
Nice 👍
|
||
**Current process** pypi upload packages from `dist/` via twine. | ||
|
||
If you want to use the scripts in `task/` you need a `.pypirc` with a correctly configured `pypi` section (see below). Otherwise just upload the release package which ever way you see fit. |
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.
Nit:
``task/``, ``.pypirc``
rst uses double back-ticks for monospace formatting
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 is true for the rest of the document as well
HOWTORELEASE.rst
Outdated
|
||
If you want to use the scripts in `task/` you need a `.pypirc` with a correctly configured `pypi` section (see below). Otherwise just upload the release package which ever way you see fit. | ||
|
||
[pypi] section in `.pypirc` should look somehow like this:: |
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.
You can use:
.. code-block:: ini
[pypi]
;repository=https://pypi.python.org/pypi
...
For syntax highlighting (you have to remove the ::
from the previous sentence).
|
||
Report bugs for tox in the `issue tracker <https://github.com/tox-dev/tox/issues>`_. | ||
|
||
If you are reporting a bug, please include: |
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.
maybe point out here that tox -v
contains most of this information, so they don't have to come up with it from the top of their head
|
||
$ tox -e docs | ||
|
||
The built documentation should be available in the ``doc/_build/``. |
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.
have we changed this, I would but it under the env, so it gets cleaned up together; aka under .tox/doc
. I prefer to keep the working directory clean, and have a single temporary/building directory, which already is .tox
#. `Fork the repository<https://help.github.com/articles/fork-a-repo/>`_ | ||
#. Make your changes. | ||
#. open a `pull request <https://help.github.com/articles/about-pull-requests/>`_ targeting the ``master`` branch. | ||
#. Follow **PEP-8**. There's a ``tox`` command to help fixing it: ``tox -e fix-lint``. |
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.
probably also mention that we have a style check tox environment that warn about non fixes, e.g. flake8
@@ -1,6 +1,6 @@ | |||
environment: | |||
matrix: | |||
- TOXENV: style | |||
- TOXENV: fix-lint |
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.
hmmm, I don't like this name, cause it does not only fixes but checks style 🤔
@@ -0,0 +1,10 @@ | |||
file ``544.feature.rst``:: | |||
|
|||
``tox --version`` now shows information about all registered plugins - by @obestwalter. |
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.
did we drop the user links or make them in another form?
tasks/pre-process-changelog.py
Outdated
|
||
|
||
def main(): | ||
manipulate_the_news() |
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.
I would also say, just alias it
[testenv:pra] | ||
description = "personal release assistant" - see HOWTORELEASE.rst | ||
skip_install = True | ||
deps = devpi |
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.
add lower version constraint for this
My current master plan would be to merge this at the weekend after releasing 2.9 and then integrating the other docs branch. I know it's a lot of churn and not very tidy, but I am willing to take the brunt of any problems that pop up by this and fixing it as we go along. |
o.k. I will merge this now and iron out the rest on master, changelog of 2.9 is broken and I hope to fix all this today and release a 2.9.1 simply to fix cosmetics. |
closes #614
These are my first efforts of getting to an easy release process that still does all the fancy stuff needed.
I extened towncrier to also link to prs and added a contrib folder with little helpers to automate releases.
towncrier pr is here: twisted/towncrier#92