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

fallout from pylint change. #1523

Merged
merged 1 commit into from
Apr 25, 2023
Merged

fallout from pylint change. #1523

merged 1 commit into from
Apr 25, 2023

Conversation

janiversen
Copy link
Collaborator

Seems that CI once again did not catch everything.

@alexrudd2 alexrudd2 changed the title fallout from flake8 change. fallout from pylint change. Apr 25, 2023
@alexrudd2
Copy link
Collaborator

The pylint cache is being preserved across versions. On the CI runner the cache is ~/.cache/pylint, which I guess gets included in the cache/restore Action.

I can reproduce locally:

  1. Run check_ci.sh (or pylint directly) on 7b585d9. (This will pass, and create/update the cache).
  2. Checkout b6384d0
  3. Run pylint again. This will pass, but it should not
  4. Clear the cache (in my case ~/Library/Caches/pylint)
  5. Run pylint again. This will fail.

@janiversen
Copy link
Collaborator Author

That's a good explanation.

@janiversen janiversen merged commit eff773a into dev Apr 25, 2023
@janiversen janiversen deleted the broad2 branch April 25, 2023 15:38
@jamesbraza
Copy link
Contributor

jamesbraza commented Apr 25, 2023

Seems that CI once again did not catch everything.

I can explain the issue here, it's a common issue when a repo doesn't require branches to be up-to-date before merging:

  1. pylint update PR pylint and pre-commit autoupdate #1519 was merged
  2. Note that ruff in Adding flake8-pytest-style to ruff #1520 made changes that pylint would have opinions about, but that branch was still using the old pylint
  3. Adding flake8-pytest-style to ruff #1520 was merged without a git rebase or a git merge
    • And its green checkmark was last attained with an old pylint and old source code
  4. Thus, when CI was next run, pylint finally was able to express opinions on ruff's changes

If pymodbus required branches to be up-to-date (a GitHub rule which I would advocate for), this issue would have surfaced within #1520 after a rebase or merge.

Hope this explains things!

@janiversen
Copy link
Collaborator Author

we actually demand pull requests to be on the newest dev.
"require branches to be up to date before merging" is checked. However I might need to add a status check.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants