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

Revert "Merge pull request #8183 from hasnainvirk/QOS_impl" #8393

Merged
merged 1 commit into from Oct 12, 2018

Conversation

Projects
None yet
5 participants
@cmonr
Contributor

cmonr commented Oct 11, 2018

Description

It was observed that after gatekeeping today, new PRs started failing. PR #8183 caused one of the tests to start failing, thus breaking master.

The following events led to this revert being needed:

  • #8183 was opened.
  • Unit tests were automatically run on the PR and passed.
  • Due to high PR and CI load, maintainers were not able to start testing the PR until a few days ago.
    • PR remained opened for over 20 days.
  • Once PR received maintainer attention, the PR passed typical maintainer-kicked CI jobs.
  • Unittests remained green, and all other tests indicated green. PR was merged today.

At some point in between when unittests were automatically run on the PR, and when it was merged, either master had moved to a point, or the test job had been updated such that the PR was now breaking unittests.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change
Revert "Merge pull request #8183 from hasnainvirk/QOS_impl"
This reverts commit 5c675d3, reversing
changes made to 2b04a02.

@cmonr cmonr added the needs: review label Oct 11, 2018

@cmonr cmonr requested review from hasnainvirk and ARMmbed/mbed-os-maintainers Oct 11, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Oct 11, 2018

@hasnainvirk Asking for your ok to bring this in, since it undoes your changes. After this, opening a new PR with the contents of #8183 would show the errors that are now present in master.

The command to test them: python ./UNITTESTS/mbed_unittest.py --compile --coverage both --clean

@0xc0170

Making master green 👍

@cmonr

This comment has been minimized.

Contributor

cmonr commented Oct 11, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 11, 2018

Build : SUCCESS

Build number : 3330
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8393/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@cmonr cmonr requested a review from SeppoTakalo Oct 11, 2018

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Oct 12, 2018

Revert instead of fixing the unittest? Have you tried to contact Hasnain or Antti?

@adbridge

This comment has been minimized.

Contributor

adbridge commented Oct 12, 2018

@SeppoTakalo Yeah Hasnain is tagged above, the problem is we need this sorted asap as all other PRs are blocked until it is. Quickest fix as Cruz said is to revert it and then one of the guys can fix it in their own time.

@adbridge adbridge merged commit 5e4b872 into ARMmbed:master Oct 12, 2018

15 checks passed

ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed , RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Passed, 655 files (+0 files)
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10367 cycles (+578 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8372B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment