-
Notifications
You must be signed in to change notification settings - Fork 109
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 format.yml and clippy.yml #135
Conversation
add nightly to the test for fmt.
update to add nightly checks.
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.
Just a simple review, I hope it will be appreciated
@@ -2,11 +2,16 @@ name: Format | |||
on: [push, pull_request] | |||
jobs: | |||
all: | |||
runs-on: ubuntu-latest | |||
runs-on: ${{ matrix.os }} |
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.
We can use ubuntu-latest
directly here since it's the only supported os
runs-on: ${{ matrix.os }} | ||
strategy: | ||
matrix: | ||
os: [ubuntu-latest] |
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.
Remove this line if the comment above is going to be accepted
@@ -2,11 +2,16 @@ name: Clippy | |||
on: [push, pull_request] | |||
jobs: | |||
all: | |||
runs-on: ubuntu-latest | |||
runs-on: ${{ matrix.os }} |
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.
Same as the first comment
runs-on: ${{ matrix.os }} | ||
strategy: | ||
matrix: | ||
os: [ubuntu-latest] |
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.
Same as the second comment
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.
yeah i remove the os part in a new PR since i had already merged this. Originally left it just in case we needed to add in windows and mac. but to keep it simple for now linux should cover most stuff and the matrix os can be added back later when needed.
add nightly to the test for fmt.