Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2007773003:
Separate out xml dependencies and tests from rtc_unittests into a separate target. (Closed)
|
Created:
4 years, 7 months ago by tommi Modified:
4 years, 7 months ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, qiang.lu, niklas.enbom, yujie_mao (webrtc), peah-webrtc Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionSeparate out xml dependencies and tests from rtc_unittests into a separate target.
NOTRY=true
(using notry due to offline android_arm64_rel bot)
Committed: https://crrev.com/32e80e4b2ec2df9449bb8b98fa1428d6e5c4aaa9
Cr-Commit-Position: refs/heads/master@{#12869}
Patch Set 1 #
Messages
Total messages: 26 (12 generated)
tommi@webrtc.org changed reviewers: + phoglund@webrtc.org
Hej Patrik - what do you think about this?
The CQ bit was checked by tommi@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2007773003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2007773003/1
In general we resist the idea of more test binaries, but I like the idea of separating out the xmpp parts so it's easier to get rid of them later. Is that the idea? We will have to make the bots run the new test binary but that's easy to fix.
On 2016/05/24 08:48:44, phoglund wrote: > In general we resist the idea of more test binaries, but I like the idea of > separating out the xmpp parts so it's easier to get rid of them later. Is that > the idea? We will have to make the bots run the new test binary but that's easy > to fix. Yes, that's the idea. I want to isolate the dependencies more. I'm also looking at downstream usage of some of our targets and see if there are some transitive xml dependencies there (e.g. with the merge_libs gyp files). If not, then we can remove those xml dependencies too and eventually separate completely from the rtc code.
ok, ship it! lgtm
The CQ bit was unchecked by tommi@webrtc.org
The CQ bit was checked by tommi@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2007773003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2007773003/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by tommi@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2007773003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2007773003/1
Description was changed from ========== Separate out xml dependencies and tests from rtc_unittests into a separate target. BUG= ========== to ========== Separate out xml dependencies and tests from rtc_unittests into a separate target. NOTRY=true (using notry due to offline android_arm64_rel bot) ==========
The CQ bit was unchecked by tommi@webrtc.org
The CQ bit was checked by tommi@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2007773003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2007773003/1
Message was sent while issue was closed.
Description was changed from ========== Separate out xml dependencies and tests from rtc_unittests into a separate target. NOTRY=true (using notry due to offline android_arm64_rel bot) ========== to ========== Separate out xml dependencies and tests from rtc_unittests into a separate target. NOTRY=true (using notry due to offline android_arm64_rel bot) ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Separate out xml dependencies and tests from rtc_unittests into a separate target. NOTRY=true (using notry due to offline android_arm64_rel bot) ========== to ========== Separate out xml dependencies and tests from rtc_unittests into a separate target. NOTRY=true (using notry due to offline android_arm64_rel bot) Committed: https://crrev.com/32e80e4b2ec2df9449bb8b98fa1428d6e5c4aaa9 Cr-Commit-Position: refs/heads/master@{#12869} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/32e80e4b2ec2df9449bb8b98fa1428d6e5c4aaa9 Cr-Commit-Position: refs/heads/master@{#12869}
Message was sent while issue was closed.
kjellander@webrtc.org changed reviewers: + kjellander@webrtc.org
Message was sent while issue was closed.
Please loop me in for CLs like this (and don't run all CQ trybots for it). Should I add xmllite_xmpp_unittests to the bots?
Message was sent while issue was closed.
On 2016/05/24 15:13:42, kjellander (webrtc) wrote: > Please loop me in for CLs like this (and don't run all CQ trybots for it). Better to hit a problem on the trybots than the build bots. What's the issue? > Should I add xmllite_xmpp_unittests to the bots? I think we should in the short term, but WebRTC doesn't use this code and it's not being maintained. So, I think we should look into moving the test code and the relevant code to where it's being used.
Message was sent while issue was closed.
On 2016/05/24 17:50:38, tommi-webrtc wrote: > On 2016/05/24 15:13:42, kjellander (webrtc) wrote: > > Please loop me in for CLs like this (and don't run all CQ trybots for it). > > Better to hit a problem on the trybots than the build bots. What's the issue? Right, but when it's possible to make the call that no tests are affected for a CL, just running a few compile bots is enough and saves us from loading the slow (Android testers) trybots. android_arm64_rel was not offline, it just had a 14 builds queue that takes about 7 hours to catch up with (+more considering more jobs are added during that time). > > Should I add xmllite_xmpp_unittests to the bots? > > I think we should in the short term, but WebRTC doesn't use this code and it's > not being maintained. So, I think we should look into moving the test code and > the relevant code to where it's being used. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
