Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2053263002:
Added a missing path separator that caused aecdump recordings not to be created in Android apprtc (Closed)
|
Created:
4 years, 6 months ago by peah-webrtc Modified:
4 years, 6 months ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionA missing path separator caused aecdump recordings
not to be created in apprtc on Android.
The path separator was missing when the path for the aecdump
file was created. This CL adds that path separator.
Note that the change of the formatting of the rest of the
line was caused by "git cl format" (the clang automatic
formatting).
BUG=webrtc:5991
Committed: https://crrev.com/81d99b304969021cb1799ed6bfc19f2792959a53
Cr-Commit-Position: refs/heads/master@{#13121}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Formatting changes according to reviewer comments #Messages
Total messages: 37 (18 generated)
Description was changed from ========== Added a missing path separator that caused aecdump recordings not to be created in apprtc on Android. The path separator was missing when the path for the aecdump file was created. This CL adds that path separator. BUG=webrtc:5991 ========== to ========== A missing path separator caused aecdump recordings not to be created in apprtc on Android. The path separator was missing when the path for the aecdump file was created. This CL adds that path separator. BUG=webrtc:5991 ==========
peah@webrtc.org changed reviewers: + sakal@webrtc.org
Description was changed from ========== A missing path separator caused aecdump recordings not to be created in apprtc on Android. The path separator was missing when the path for the aecdump file was created. This CL adds that path separator. BUG=webrtc:5991 ========== to ========== A missing path separator caused aecdump recordings not to be created in apprtc on Android. The path separator was missing when the path for the aecdump file was created. This CL adds that path separator. Note that the change of the formatting of the rest of the line was caused by "git cl format" (the clang automatic formatting). BUG=webrtc:5991 ==========
Hi, Could you please review this CL? I chose you as a reviewer as you have lately been working with this part of the code.
The git cl format works incorrectly for our project. It should not be used. Specifically, it uses 4 space indentation where as we use 2 space indentation. Otherwise, the change looks good. I replaced the hard coded /sdcard/ path without testing it, shame on me. https://codereview.webrtc.org/2053263002/diff/1/webrtc/examples/androidapp/sr... File webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java (right): https://codereview.webrtc.org/2053263002/diff/1/webrtc/examples/androidapp/sr... webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java:514: aecDumpFileDescriptor = ParcelFileDescriptor.open( Please use 2 space indentation instead of 4 space indentation.
Patchset #2 (id:20001) has been deleted
Great! Thanks! I have now reformatted the code according to how it was before the CL. PTAL
lgtm
The CQ bit was checked by peah@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2053263002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_arm64_rel/build...) android_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/14046)
The CQ bit was checked by peah@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2053263002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: presubmit on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/6196)
peah@webrtc.org changed reviewers: + perkj@webrtc.org
+perkj@ as an OWNER
The CQ bit was checked by perkj@webrtc.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2053263002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_arm64_rel/build...) android_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/14103)
The CQ bit was checked by sakal@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2053263002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/14113)
The CQ bit was checked by sakal@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2053263002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/14129)
The CQ bit was checked by sakal@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2053263002/40001
Message was sent while issue was closed.
Description was changed from ========== A missing path separator caused aecdump recordings not to be created in apprtc on Android. The path separator was missing when the path for the aecdump file was created. This CL adds that path separator. Note that the change of the formatting of the rest of the line was caused by "git cl format" (the clang automatic formatting). BUG=webrtc:5991 ========== to ========== A missing path separator caused aecdump recordings not to be created in apprtc on Android. The path separator was missing when the path for the aecdump file was created. This CL adds that path separator. Note that the change of the formatting of the rest of the line was caused by "git cl format" (the clang automatic formatting). BUG=webrtc:5991 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== A missing path separator caused aecdump recordings not to be created in apprtc on Android. The path separator was missing when the path for the aecdump file was created. This CL adds that path separator. Note that the change of the formatting of the rest of the line was caused by "git cl format" (the clang automatic formatting). BUG=webrtc:5991 ========== to ========== A missing path separator caused aecdump recordings not to be created in apprtc on Android. The path separator was missing when the path for the aecdump file was created. This CL adds that path separator. Note that the change of the formatting of the rest of the line was caused by "git cl format" (the clang automatic formatting). BUG=webrtc:5991 Committed: https://crrev.com/81d99b304969021cb1799ed6bfc19f2792959a53 Cr-Commit-Position: refs/heads/master@{#13121} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/81d99b304969021cb1799ed6bfc19f2792959a53 Cr-Commit-Position: refs/heads/master@{#13121} |
