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

GitHub Action to lint Python code #258

Closed
wants to merge 1 commit into from
Closed

Conversation

cclauss
Copy link

@cclauss cclauss commented Jan 8, 2025

The googletest vendored into this codebase was never ported to Python 3, so it has at least 64 files containing Python Syntax Errors. It should be re-vendored or removed if it is no longer used.

* https://docs.github.com/en/actions
* https://docs.github.com/en/actions/use-cases-and-examples/building-and-testing/building-and-testing-python
* https://docs.astral.sh/ruff

The `googletest' vendored into this codebase was never ported to Python 3, so it has at least 64 files containing Python Syntax Errors.
@mwallnoefer
Copy link
Collaborator

First of all, thanks for the contribution. I have managed to run it over the merge functionality and it catches all those Python 3 incompatibilities mentioned by you.

The point I wanted to raise is that this code base is pretty much maintenance-only. If I have understood it correctly, accepting this PR requests us to sort out all those Python 3 incompatibilities, otherwise we may not accept other fixes/patches respectively PRs.

I am not sure if it is really worth it, given that the codebase is old (e.g. we do also not support Mqtt 5.x), so I put also the other maintainers in the CC to hear their opinions.

@ejvr , @KonstantinRitt What do you think?

@ejvr
Copy link
Contributor

ejvr commented Jan 8, 2025

We're not using python for anything in this repo, as far as I know. If all offending files are part of googletest, I'd propose we either update those files to the latest version of googlet test, of remove the code (if someone wants to run the test, the test app should be linked agaoinst an existing external library).

@cclauss cclauss closed this Jan 9, 2025
@cclauss cclauss deleted the patch-1 branch January 9, 2025 06:08
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.

3 participants