-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Travis: remove status functionality #13619
Conversation
@0xc0170, thank you for your changes. |
I'll run some checks (astyle failures ,etc to see how it goes with the regular status updates). |
975201c
to
2d4e9f3
Compare
To make this PR green , I would need to remove "required status checks for travis-ci/*". The Travis provides a report for each job: https://travis-ci.org/github/ARMmbed/mbed-os/builds/727646560 - this should be sufficient to review after the failure. @ARMmbed/mbed-os-test @ARMmbed/mbed-os-maintainers Please review |
This PR cannot be merged due to conflicts. Please rebase to resolve them. |
2d4e9f3
to
7eb5a51
Compare
Rebased, testing now 2 new commits if this works as expected after the recent change to travis. |
10cfed3
to
7eb5a51
Compare
Unfortunately, we need to remove status functionality as it exposes an information it should rather not. There is no other way we can do it in the pull request coming from forks (most of our PRs are from forks). It is better if we use pass/failure what Travis provides. The information are in the logs. It should always print the status info at the very end. A test should have "echo Failed with a reason...."
7eb5a51
to
14714a8
Compare
I pushed an update, to fix OK license status for Travis. |
@harmut01 if I do not manage to get this green, could you take over to fix the license check and remove statuses from PRs ? |
Travis green, looks like it is working now. However all Expected in travis should be removed from Github required statuses @jamesbeyond @adbridge Please review |
@0xc0170 how will we know going forward if a PR has failed travis checks if they are not 'required' ? |
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 as far as I can tell
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.
This looks good to me, though I am very familiar with the TravisCI side of code,
There is one main Travis check - if you click , it will show you the matrix of jobs. So basically the same just one more click away, no test coverage loosing. Just this is the way it is supported by Travis |
CI started |
The required statuses cleaned - so pr head and travis only |
Summary of changes
Unfortunately, we need to remove status functionality as it exposes an information
it should rather not. There is no other way we can do it in the pull request coming
from forks (most of our PRs are from forks). It is better if we use pass/failure what
Travis provides. The information are in the logs. It should always print the status info at
the very end. A test should have "echo Failed with a reason...."
Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers