Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1486423003:
Fix error prone code in VideoCapturerAndroid (Closed)
|
Created:
5 years ago by perkj_webrtc Modified:
5 years ago Reviewers:
magjed_webrtc CC:
webrtc-reviews_webrtc.org Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionFix error prone code in VideoCapturerAndroid
BUG=webrtc:5282
Committed: https://crrev.com/672aba3f57061e33dd802d9a391c54bdfed952c3
Cr-Commit-Position: refs/heads/master@{#11046}
Patch Set 1 : #
Total comments: 2
Patch Set 2 : Fix comment #Patch Set 3 : Rebased #
Messages
Total messages: 21 (9 generated)
Patchset #1 (id:1) has been deleted
perkj@webrtc.org changed reviewers: + magjed@webrtc.org
please?
https://codereview.webrtc.org/1486423003/diff/20001/talk/app/webrtc/java/andr... File talk/app/webrtc/java/android/org/webrtc/CameraEnumerationAndroid.java (right): https://codereview.webrtc.org/1486423003/diff/20001/talk/app/webrtc/java/andr... talk/app/webrtc/java/android/org/webrtc/CameraEnumerationAndroid.java:204: final int MAX_FPS_WEIGHT = 10; Has this anything to do with bug=5282? You should name it |maxFpsWeight| instead of |MAX_FPS_WEIGHT|: "Even when final and immutable, local variables are not considered to be constants, and should not be styled as constants."
On 2015/12/03 11:07:35, magjed_webrtc wrote: > https://codereview.webrtc.org/1486423003/diff/20001/talk/app/webrtc/java/andr... > File talk/app/webrtc/java/android/org/webrtc/CameraEnumerationAndroid.java > (right): > > https://codereview.webrtc.org/1486423003/diff/20001/talk/app/webrtc/java/andr... > talk/app/webrtc/java/android/org/webrtc/CameraEnumerationAndroid.java:204: final > int MAX_FPS_WEIGHT = 10; > Has this anything to do with bug=5282? You should name it |maxFpsWeight| instead > of |MAX_FPS_WEIGHT|: > "Even when final and immutable, local variables are not considered to be > constants, and should not be styled as constants." It is a comment in the same code review that glaznev created this defect from. But it is not mentioned in the defect.
ptal https://codereview.webrtc.org/1486423003/diff/20001/talk/app/webrtc/java/andr... File talk/app/webrtc/java/android/org/webrtc/CameraEnumerationAndroid.java (right): https://codereview.webrtc.org/1486423003/diff/20001/talk/app/webrtc/java/andr... talk/app/webrtc/java/android/org/webrtc/CameraEnumerationAndroid.java:204: final int MAX_FPS_WEIGHT = 10; On 2015/12/03 11:07:35, magjed_webrtc wrote: > Has this anything to do with bug=5282? You should name it |maxFpsWeight| instead > of |MAX_FPS_WEIGHT|: > "Even when final and immutable, local variables are not considered to be > constants, and should not be styled as constants." Done.
lgtm
The CQ bit was checked by perkj@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1486423003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1486423003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_gn_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_gn_rel/builds/7679) ios32_sim_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/3987) ios_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/11465) mac_asan on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/11313)
The CQ bit was checked by perkj@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org Link to the patchset: https://codereview.webrtc.org/1486423003/#ps60001 (title: "Rebased")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1486423003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1486423003/60001
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) linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL) mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL) win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by perkj@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1486423003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1486423003/60001
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Fix error prone code in VideoCapturerAndroid BUG=webrtc:5282 ========== to ========== Fix error prone code in VideoCapturerAndroid BUG=webrtc:5282 Committed: https://crrev.com/672aba3f57061e33dd802d9a391c54bdfed952c3 Cr-Commit-Position: refs/heads/master@{#11046} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/672aba3f57061e33dd802d9a391c54bdfed952c3 Cr-Commit-Position: refs/heads/master@{#11046} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
