-
Notifications
You must be signed in to change notification settings - Fork 6
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
Rewrite CI to a Modern Standard #21
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! A quick question:
16d8e43
to
1ceb7fc
Compare
e53a6b5
to
fd1d003
Compare
There seems to be a problem with |
a6b541c
to
98f35d3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Here are a few things I noticed while just reading over the changes. I'd like to test some of the more suspicious changes to make sure these won't break crucial functionality (even though they shouldn't), so this might not be the full review.
Thanks for the review! |
You're welcome! It's quite sad that it fails for my hacky I think what you do with |
I just went ahead and deleted it entirely :) |
On second thought, I just had an idea for a script making sure that |
This is due to the removed instruction `pipenv install --dev --deploy` which normally created the correct files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! Are you good with merging this in, or are you planning more changes?
I think I'm good with this! |
Motivation
Tin's CI is kind of weak, with a mix of
black
,autopep8
andisort
that requires you to run the script before committing. Instead, we can use tools like pre-commit to auto-run these checks before committing, reducing the mental load on developers. Tin has a primitive form ofpre-commit
in place, but to utilize the concurrent nature of pre-commit it's better to use different hooks.Also, the current script being run is very lax, more rules would result in better code quality with less mental load on developers.
TL;DR I hate running scripts.
Revamp CI
scripts/format.sh
scripts/check.sh
Linting
Bonus Request