Conversation
4d72763 to
27750ce
Compare
9ec6880 to
655b50d
Compare
655b50d to
1e398c8
Compare
| - name: "Lint everything and check for tidy dependencies" | ||
| run: "go run magefile.go lint:all && go run magefile.go deps:tidy" |
There was a problem hiding this comment.
Why is this chained as an && instead of using 2 sequential run operations?
| with: | ||
| path: "" | ||
| fixup-command: "go run magefile.go deps:tidy" | ||
| fixup-command: "go run magefile.go lint:all && go run magefile.go deps:tidy" |
There was a problem hiding this comment.
This ran already above, why does it need to run again?
| jobs: | ||
| go-lint: | ||
| name: "Lint Go" | ||
| lint: |
There was a problem hiding this comment.
One thing to keep an eye on is that we are now running a more expensive lint job, whereas before we ran multiple concurrent lint operations. This means we are trading off smaller, faster jobs for one larger, more expensive job. That impacts the feedback loop (how long it takes for the developer to receive feedback from CI). This may not be a problem, but please keep an eye on the timing of these jobs.
There was a problem hiding this comment.
I doubt anyone uses this, but this would be a breaking change, right?
| } | ||
|
|
||
| // Integration runs the integration tests | ||
| func (Test) Integration() error { |
There was a problem hiding this comment.
The unit tests have a timeout of 20m, but the integration tests have a timeout of 1m? No race detector?
Description
Testing
Checkout the PR locally and ensure all the commands run successfully.