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

Linting workflow & Dependabot integration #17

Merged
merged 5 commits into from
Oct 9, 2023
Merged

Linting workflow & Dependabot integration #17

merged 5 commits into from
Oct 9, 2023

Conversation

ToasterUwU
Copy link
Contributor

As promised in #13

@ToasterUwU ToasterUwU marked this pull request as ready for review October 9, 2023 18:00
@ToasterUwU
Copy link
Contributor Author

@rsnitsch Have a look, and feel free to merge if you are ok with this.

I use this exact same thing in many of my Python projects, it helps a lot to minimize upkeep and also prevents many of the basic issues that can arise ( unbound vars, wrong types, etc. )

@ToasterUwU
Copy link
Contributor Author

In case you arent familiar, everything used in here is free to use, so even tho this takes processing power, Github doesnt charge you for it.

@ToasterUwU ToasterUwU marked this pull request as draft October 9, 2023 18:04
@ToasterUwU
Copy link
Contributor Author

Just realized i need to change it to use pipenv instead ( I don't use that )

.github/workflows/lint.yml Outdated Show resolved Hide resolved
@rsnitsch
Copy link
Owner

rsnitsch commented Oct 9, 2023

Very promising, thank you.

@ToasterUwU ToasterUwU marked this pull request as ready for review October 9, 2023 18:35
@ToasterUwU
Copy link
Contributor Author

Ok, i have never used pipenv, i need to test it really quick on my fork, just to make sure it runs fine. After that you can merge it.

@rsnitsch
Copy link
Owner

rsnitsch commented Oct 9, 2023

Ok, i have never used pipenv, i need to test it really quick on my fork, just to make sure it runs fine. After that you can merge it.

Thank you for the extra diligence. 👌

@ToasterUwU
Copy link
Contributor Author

@rsnitsch Take another look and merge if you want to. I tested this on my fork, and it worked.
Meaning it screamed at me for the fact. I don't have the fixes from the #16 PR, but with them its smooth sailing.

@ToasterUwU
Copy link
Contributor Author

If you wonder which to merge first, doesn't really matter.

The cleaner one would be to merge the other one first and then this one, but in the end of the day it will show "Lint Passed" as the newest status anyways.

The actual PR for #13 I will make either tomorrow or the day after, depending on what else is on my list tommorow.

@rsnitsch rsnitsch merged commit c813a3f into rsnitsch:develop Oct 9, 2023
@rsnitsch
Copy link
Owner

rsnitsch commented Oct 9, 2023

Thank you very much! I merge this first because I want to see it screaming. ;-)

@ToasterUwU ToasterUwU deleted the feat/github-workflows branch October 9, 2023 21:07
@ToasterUwU
Copy link
Contributor Author

@rsnitsch Btw, it might make sense to add all Python versions you want to officially support to the lint workflow. Just add them to the OS matrix thing next to 3.10 and 3.11. I recommend putting 3.8 in at least because that's pre-built-in type hinting.

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