Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2033573002:
[Chromoting] Final SelfRemovableParameterRunnable (Closed)
Description[Chromoting] Final SelfRemovableParameterRunnable
This is a trivial change to make Event.SelfRemovableParameterRunnable final.
BUG=615277
Committed: https://crrev.com/90227c12f3bd94c48c6aa4bccb2ce773a4c48ecb
Cr-Commit-Position: refs/heads/master@{#397769}
Patch Set 1 #
Messages
Total messages: 19 (9 generated)
Description was changed from ========== Final SelfRemovableParameterRunnable BUG=615277 ========== to ========== [Chromoting] Final SelfRemovableParameterRunnable This is a trivial change to make Event.SelfRemovableParameterRunnable final. BUG=615277 ==========
zijiehe@chromium.org changed reviewers: + lambroslambrou@chromium.org
Is there a reason to prevent subclassing here? I believe Chrome generally discourages inheritance anyway. lgtm anyway - I'm not aware of any guidance on the use of 'final' in Chromium.
On 2016/06/02 17:24:08, Lambros wrote: > Is there a reason to prevent subclassing here? > I believe Chrome generally discourages inheritance anyway. > > lgtm anyway - I'm not aware of any guidance on the use of 'final' in Chromium. I think it's a good habit disallow inheritance or override if we do not need it or intent to do it for now. Then any inheritance attempts would be controlled by us -- at least he needs to change this file :) --, which can help to avoid unexpected inheritance and potential maintenance cost. For this change, well, considering it's a class inner private static class, final or not seems not so important.
The CQ bit was checked by zijiehe@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2033573002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2033573002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by zijiehe@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2033573002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2033573002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by zijiehe@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2033573002/1
Message was sent while issue was closed.
Description was changed from ========== [Chromoting] Final SelfRemovableParameterRunnable This is a trivial change to make Event.SelfRemovableParameterRunnable final. BUG=615277 ========== to ========== [Chromoting] Final SelfRemovableParameterRunnable This is a trivial change to make Event.SelfRemovableParameterRunnable final. BUG=615277 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== [Chromoting] Final SelfRemovableParameterRunnable This is a trivial change to make Event.SelfRemovableParameterRunnable final. BUG=615277 ========== to ========== [Chromoting] Final SelfRemovableParameterRunnable This is a trivial change to make Event.SelfRemovableParameterRunnable final. BUG=615277 Committed: https://crrev.com/90227c12f3bd94c48c6aa4bccb2ce773a4c48ecb Cr-Commit-Position: refs/heads/master@{#397769} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/90227c12f3bd94c48c6aa4bccb2ce773a4c48ecb Cr-Commit-Position: refs/heads/master@{#397769} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
