Conversation
static tests are already executed by Travis. Failing Travis will already block the merge, so we don't have to run those tests in Murdock again. This has two benefits: - slightly decreased CI build times - Murdock failures will only be caused by build errors. The last point is my main motivation for this patch. It often happens that Murdock failures are dismissed during review if a PR is not squashed yet. Even if there are still build failures, they are not visible at a glance. Travis will already fail when the PR is not yet squashed, so Murdock doesn't have to check this again and can now only warn about serious issues.
Not really, Travis is not required to be green in the project configuration but that can be changed. +1 for this PR. |
| fi | ||
|
|
||
| BUILDTEST_MCU_GROUP=static-tests ./dist/tools/ci/build_and_test.sh | ||
| # Travis will already run static tests |
There was a problem hiding this comment.
Why not remove the whole function static_tests() altogether?
There was a problem hiding this comment.
BTW... I think this skips the rebase step included in the static tests.
There was a problem hiding this comment.
It seems like the function also takes care of cloning the repo.
TBH I'm not very familiar with the inner workings of Murdock, so my bet was to invoke Cunningham's Law on this one.
There was a problem hiding this comment.
It seems like the function also takes care of cloning the repo.
Not entirely. The static tests script (./dist/tools/ci/build_and_test.sh) tests if the current branch can be rebased on master. It does so by rebaseing on upstream/master.
"static_tests()" just ensures that there's an upstream and it is correctly fetched.
How do you mean? Murdock build failures are visible in the last PR build result. Or do you mean possible warnings in the static test? |
|
This also skips the static tests in nightlies. I don't like it, I prefer a single source of authority. |
If the PR contains !fixup commits, Murdock failure is to be expected, so often a Murdock failure is ignored even though it was caused by a build failure. |
Isn't that basically more of a graphic design issue? We could just make the compile errors more prominent (and not put them to the very end of |
Sure!
I'm talking about this interface:
If you expect the Murdock failure to be just because of !fixups, you don't click the link to get more details. Another option would be to make Murdock reply to the PR directly. |
I've been thinking of adding more fine grained PR statuses. I was mostly concerned with warnings, which are currently easily ignored. This needs some setup, but we could add anything here. |
Ok, but if you don't even look at the results and expect everything to be perfect then its a PEBCAK.
It kind of does already in the status. If the build fails it says "the build failed", if only the static tests fail it says "static tests failed" IIRC. |
Contribution description
Static Tests are already executed by Travis.
Failing Travis will already block the merge, so we don't have to run those tests in Murdock again.
This has two benefits:
The last point is my main motivation for this patch.
It often happens that Murdock failures are dismissed during review if a PR is not squashed yet.
Even if there are still build failures, they are not visible at a glance.
Travis will already fail when the PR is not yet squashed, so Murdock doesn't have to check this again and can now only warn about serious issues.
Testing procedure
Issues/PRs references
none