-
Notifications
You must be signed in to change notification settings - Fork 267
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
Update formatting script and use GitHub Actions #200
base: master
Are you sure you want to change the base?
Conversation
af404d7
to
82aa96e
Compare
Should this be rebased to fix the new formatting errors? |
82aa96e
to
56abc66
Compare
@UebelAndre Thanks for the ping, I updated the PR. If it needs updating in the future, let me know. |
I think it's up to @richgel999 but I think it'd be good to separate linting from the build |
Does this require any manual action to enable? Or are GitHub actions enabled automatically based on something in the repo? |
@UebelAndre It should be automatically enabled, but there are also repo settings to turn it on and off. |
Just wondering if there's any info @richgel999 should have when should this PR get merged to make sure it's properly enabled |
@richgel999 hey, do you have a sec to look at this? It'd be really nice to separate the linting from the Linux build so it's more obvious what's wrong and when |
Yes, I'm working down the PR list. Please Standby. |
56abc66
to
cbd20d0
Compare
I will run the formatting script on v1.16 - the January release. |
cbd20d0
to
e7c3969
Compare
e7c3969
to
7ab6e00
Compare
@richgel999 @UebelAndre I know it's been a long time, but I updated the PR to use Python and pre-commit instead of a shell script. Compared to the old version of this PR, it gives the following advantages:
|
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 to me!
This is a follow-up to #115 and #136. Over time I've made improvements to this script, so this PR updates it in this repo.
The
tail
command has been replaced withperl
, which is more portable, as it also runs on macOS. There is also a set pipefail and IFS definition that properly define those for any OS that doesn't already do so. I also added a.gitattributes
file, which tells Git to resolve line ending encoding problems automatically.Aside from the script, the CI has been updated to use GitHub Actions. This is nicer than Travis or AppVeyor, the status and details are nicely integrated into GitHub, and if there are multiple jobs they can all be displayed individually. The CI won't run on this PR because it only runs if the config is already in the repo, but it will run after this PR is merged.
The second commit in this PR is just running the script, which removes many useless trailing spaces and tabs.