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

Introduce pylint ci task #35

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Introduce pylint ci task #35

merged 1 commit into from
Nov 12, 2024

Conversation

fjahr
Copy link
Collaborator

@fjahr fjahr commented Nov 11, 2024

This also fixes a few complaints from pylint but explicitly ignores many others that did not seem essential for the start. These can be fixed as we go along.

None of the code changes here should change behavior.

This also fixes a few complaints from pylint but explicitly ignores many others that did not seem essential for the start. These can be fixed as we go along.
@fjahr
Copy link
Collaborator Author

fjahr commented Nov 11, 2024

Checked that the ci task works (i.e. fails when it should) by reintroducing an error: https://github.com/asmap/kartograf/actions/runs/11786686736/job/32830471920

@fjahr fjahr mentioned this pull request Nov 11, 2024
4 tasks
@jurraca
Copy link
Contributor

jurraca commented Nov 12, 2024

ACK. more an fyi, this doesn't include the pylint package in the dev environment. You'd have to split out the dev deps like in #26, so it's probably better to add it in that one or another PR.

@fjahr
Copy link
Collaborator Author

fjahr commented Nov 12, 2024

this doesn't include the pylint package in the dev environment

Yeah, but we don't use it as part of the test suite or anything like that and we don't have a lint task/script for local dev work. So as long as we only have it in CI I wouldn't put it there since the CI can do it's own setup. But we can do a linter task/script for running it locally more easily as a follow-up.

@fjahr fjahr merged commit 127be34 into master Nov 12, 2024
4 checks passed
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.

2 participants