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

Fix Github Actions #347

Merged
merged 4 commits into from
May 28, 2024
Merged

Fix Github Actions #347

merged 4 commits into from
May 28, 2024

Conversation

rschwebel
Copy link
Contributor

Since #338, github actions are unhappy. Fix it.

To be able to manually run a workflow from the Github web interface, add
a workflow_dispatch trigger.

Note that this does only work once this change has hit the default
branch.
Python 2.7 is long obsolete, deprecated and not supported any more.
Remove it from the tests. This test job is especially supposed to run on
ubuntu-20.04, which doesn't have Python 2.7 any more.

Without this patch, this workflow fails with:

  Warning: The support for python 2.7 will be removed on June 19. Related issue: actions/setup-python#672
  Version 2.7 was not found in the local cache
  Error: The version '2.7' with architecture 'x64' was not found for Ubuntu 20.04.
  The list of all available versions can be found here: https://raw.githubusercontent.com/actions/python-versions/main/versions-manifest.json
Flake8 is unhappy with us because we violate PEP8:

rsc@leda:~/git/nodeenv$ flake8 --extend-ignore=E127 nodeenv.py tests setup.py
setup.py:16:1: E402 module level import not at top of file

Ignore this warning in this case.
According to this flake8 error:

(env) rsc@leda:~/git/nodeenv$ flake8 --extend-ignore=E127 nodeenv.py tests setup.py
nodeenv.py:421:19: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`

we should not compare types but use isinstance() instead, which can
handle subclasses as well.

See https://www.flake8rules.com/rules/E721.html for details.
@rschwebel
Copy link
Contributor Author

Cc: += @flying-sheep

@flying-sheep
Copy link
Contributor

flying-sheep commented Dec 20, 2023

I don’t think that my PR introduced any changes that resulted in breakage.

  1. checks were green for my PR, otherwise it wouldn’t have been merged
  2. my PR didn’t touch the line the linter complains about, which is this one:
    if is_PY3 and type(content) != bytes:
  3. You don’t use a pinned version of your linter:

So what happened is

  1. my PR’s checks ran successfully
  2. flake8 updated to a version that had a new check that this codebase is failing. From now on, CI runs use the new flake8 version with that check and fail
  3. my PR got merged

I suggest switching to pre-commit.ci, that way your linter versions are pinned, and you get weekly PRs to update your linters. You can then fix newly introduced lint checks at your leisure in that weekly PR instead of having them be introduced while merging some completely unrelated PR

This was referenced Dec 20, 2023
@ekalinin ekalinin merged commit bbffd9b into ekalinin:master May 28, 2024
@ekalinin
Copy link
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants