Reorder CI, make it fail fast #2138
Conversation
|
I would actually prefer if both always ran (similar to the tidy job on rust-lang/rust). As it's otherwise quite annoying to see CI fail for formatting reason and then wait again for actual tests fail. Better I think to show both failure at once. Maybe we can add |
|
IMO it makes more sense to run the easiest to fix (and run) things last. You don't want to get a formatting failure from CI if there is also a test failure. |
No opinion on the ordering of the steps. I having both errors at "the same time" is still nice IMO. |
|
The idea behind contribution is that opening a PR and having an immediate CI error gives the contributor feedback quicker, gives them the opportunity to fix it right there and then, and making them more vigilant for the next commit. Also, saving CI time. But yeah, having both run at the same time is quite nice, and seeing the reasonable feedback, I'll change the PR. |
|
I think that the more important thing is to take a look at which CI steps fail more often, and how they affect one another. If you have both a formatting and a test fail, you always want to fix the test first, because that will also require another reformat. Fixing a formatting fail just to push and see a test failure is a waste of time. In general, formatting is always the last thing that you want to do, so it should also run last on CI, IMO. Most people also have autoformatting in their IDE, so it rarely fails, and thus should run towards the end of the CI job. But for triagebot specifically, I don't think it really matters, so I will stop discussing this, as it's just bikeshedding and a waste of time, IMO :) Feel free to merge if you want. |
|
Closing this being that it's too controversial for a 2 line change. More trouble than worth |
The test suite takes about 1m 42 seconds to run, while
cargo fmt --checktakes less than a second. This PR reorders them