search

ekalinin / nodeenv

Virtual environment for Node.js & integrator with virtualenv
http://ekalinin.github.io/nodeenv/
Other
1.7k stars 209 forks source link

Fix Github Actions #347

Closed rschwebel closed 4 months ago

rschwebel commented 9 months ago

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

rschwebel commented 9 months ago

Cc: += @flying-sheep

flying-sheep commented 9 months ago

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: https://github.com/ekalinin/nodeenv/blob/eaa9de97e561ab4f99458c94633e92547e72d5f1/nodeenv.py#L421
  3. You don’t use a pinned version of your linter: https://github.com/ekalinin/nodeenv/blob/eaa9de97e561ab4f99458c94633e92547e72d5f1/requirements-dev.txt#L4

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

ekalinin commented 4 months ago

Thanks!