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

Run check-manifest and other linters via GitHub Actions. #396

Merged
merged 12 commits into from
Aug 17, 2022
8 changes: 6 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,14 @@ jobs:
# increase in stability over using the latest minor.
- name: CPython 3.9
tox: py39
action: 3.9
python-version: '3.9'
task:
- name: Check Newsfragment
tox: check-newsfragment
- name: Check package manifest
tox: check-manifest
- name: Check pre-commit
tox: pre-commit

steps:
- uses: actions/checkout@v3
Expand All @@ -182,7 +186,7 @@ jobs:
- name: Set up ${{ matrix.python.name }}
uses: actions/setup-python@v3
with:
python-version: ${{ matrix.python.action }}
python-version: ${{ matrix.python.python-version }}

- name: Install dependencies
run: python -m pip install --upgrade pip tox
Expand Down
1 change: 0 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ repos:
rev: 4.0.1
hooks:
- id: flake8
language_version: python3.10

- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.2.0
Expand Down
27 changes: 14 additions & 13 deletions CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Want to contribute to this project? Great! We'd love to hear from you!

As a developer and user, you probably have some questions about our
project and how to contribute.
In this article we try to answer these and give you some recommendations.
In this article, we try to answer these and give you some recommendations.


Ways to communicate and contribute
Expand All @@ -28,10 +28,11 @@ There are several options to contribute to this project:
If you found a bug or have a new cool feature, describe your findings.
Try to be as descriptive as possible to help us understand your issue.

* Check out the Freenode ``#twisted-dev`` IRC channel.
* Check out the Libera ``#twisted`` IRC channel or `Twisted Gitter <https://gitter.im/twisted/twisted>`_.

If you prefer to discuss some topics personally, you may find the IRC
channel interesting.
If you prefer to discuss some topics personally,
you may find the IRC or Gitter channels interesting.
They are bridged.

* Modify the code.

Expand Down Expand Up @@ -68,12 +69,14 @@ We recommend the following workflow:
*Running the test suite* for details.


c. Document any user facing changes in one of the `/docs/` files.
c. Document any user-facing changes in one of the ``/docs/`` files.

d. Create a newsfragment in ``src/towncrier/newsfragments/`` describing the changes and containing information that are of interest for end-users.
d. Create a newsfragment in ``src/towncrier/newsfragments/`` describing the changes and containing information that is of interest to end-users.

e. Create a `pull request`_.
Describe in the pull request what you did and why. If you have open questions, ask.
Describe in the pull request what you did and why.
If you have open questions, ask.
(optional) Allow team members to edit the code on your PR.

#. Wait for feedback. If you receive any comments, address these.

Expand All @@ -98,7 +101,7 @@ The following list contains some ways how to run the test suite:
You may want to add the ``--skip-missing-interpreters`` option to avoid errors
when a specific Python interpreter version couldn't be found.

* To get a complete list about the available targets, run::
* To get a complete list of the available targets, run::

$ tox -av

Expand All @@ -108,15 +111,13 @@ The following list contains some ways how to run the test suite:
$ tox -- towncrier.test.test_project.InvocationTests.test_version

* To run some quality checks before you create the pull request,
we recommend to use this call::
we recommend using this call::

$ tox -e check-manifest,check-newsfragment
$ tox -e pre-commit,check-manifest,check-newsfragment

* Install `pre-commit <https://pre-commit.com/>`_ and activate::
* Or enable `pre-commit` as a git hook::

$ pip install pre-commit
$ pre-commit
Or run as git hook
$ pre-commit install

* To investigate and debug errors, use the ``trial`` command like this::
Expand Down
3 changes: 3 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[flake8]
# Allow for longer test strings. Code is formatted to 88 columns by Black.
max-line-length = 99
Copy link
Member

Choose a reason for hiding this comment

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

I'm still in the boat of using the program's default config file. Even when using the 'single config file for everything' option you still end up with multiple single config files.

Copy link
Member Author

Choose a reason for hiding this comment

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

The plan it to move more things from setup.py into setup.cfg ... I am using it in #398

Copy link
Member

Choose a reason for hiding this comment

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

Any project using black will still have to have a pyproject.toml for any config there, afaik. Anyways, not a bug, just a passing opinion.

Empty file.
22 changes: 2 additions & 20 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[tox]
envlist = cov-erase, pre-commit, {pypy37,pypy38,py37,py38,py39,py310}-tests, flake8, check-manifest, check-newsfragment, cov-report
envlist = cov-erase, pre-commit, {pypy37,pypy38,py37,py38,py39,py310}-tests, check-manifest, check-newsfragment
isolated_build=true
skip_missing_envs = true

Expand Down Expand Up @@ -33,6 +33,7 @@ commands =
# specifying the entry point script in `{envbindir}`.
coverage run -p --module twisted.trial {posargs:towncrier}
coverage combine -a
coverage report

[testenv:build]
Copy link
Member

Choose a reason for hiding this comment

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

Why is this going away?

Copy link
Member Author

Choose a reason for hiding this comment

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

my bad. I thought that we don't need it and that it was already moved to GitHub CI only.

I will revert...

My hope was that we can use standard tools without the need of extra tools.

Copy link
Member

Choose a reason for hiding this comment

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

I wasn't sure where you were going so maybe it isn't needed now? It was an honest question.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is still need.
The towncrier is built once.
Then the wheel is uploaded as an artifact.
Then the tests will download the artifact and run the tests agains it.


I am not convinced that we need all this dance... but I think is better to refactor this in another PR.

allowlist_externals =
Expand All @@ -48,22 +49,3 @@ skip_install = true
commands =
# could be brought inside tox.ini after https://github.com/tox-dev/tox/issues/1571
bash {toxinidir}/tox_build.sh

[testenv:cov-report]
deps =
coverage
skip_install = true
commands =
coverage html
coverage report

[testenv:cov-erase]
deps =
coverage
skip_install = true
commands =
coverage erase

[flake8]
# Allow for longer test strings. Code is formatted to 88 columns by Black.
max-line-length = 99