Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2894623004:
[ObjC ARC] Converts ios/chrome/browser/voice:voice to ARC. (Closed)
|
Created:
3 years, 7 months ago by lindsayw Modified:
3 years, 6 months ago Reviewers:
kkhorimoto CC:
chromium-reviews, marq+watch_chromium.org, ios-reviews+chrome_chromium.org, noyau+watch_chromium.org, ios-reviews_chromium.org, pkl (ping after 24h if needed) Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[ObjC ARC] Converts ios/chrome/browser/voice:voice to ARC.
Automatically generated ARCMigrate commit
Notable issues:None
BUG=624363
TEST=None
Review-Url: https://codereview.chromium.org/2894623004
Cr-Commit-Position: refs/heads/master@{#476745}
Committed: https://chromium.googlesource.com/chromium/src/+/a60bca2c4ac7f84008c5345db7ba08561a0eb6bc
Patch Set 1 #
Total comments: 12
Patch Set 2 : Addressed all comments. #
Total comments: 2
Patch Set 3 : Removed curly braces. #
Messages
Total messages: 29 (18 generated)
The CQ bit was checked by lindsayw@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lindsayw@chromium.org changed reviewers: + stkhapugin@chromium.org
Thanks for the review!
Thanks for the review!
lindsayw@chromium.org changed reviewers: + kkhorimoto@chromium.org
kkhorimoto@chromium.org: Please review changes in Hi Kurt, Do you mind reviewing since Paris is out? Thanks!
Description was changed from ========== [ObjC ARC] Converts ios/chrome/browser/voice:voice to ARC. Automatically generated ARCMigrate commit Notable issues:None BUG=624363 TEST=None ========== to ========== [ObjC ARC] Converts ios/chrome/browser/voice:voice to ARC. Automatically generated ARCMigrate commit Notable issues:None BUG=624363 TEST=None ==========
lindsayw@chromium.org changed reviewers: - stkhapugin@chromium.org
https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... File ios/chrome/browser/voice/speech_input_locale_match_config.mm (right): https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... ios/chrome/browser/voice/speech_input_locale_match_config.mm:27: } This whole block within the curly braces can be removed since we can just synthesize the variable now. https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... ios/chrome/browser/voice/speech_input_locale_match_config.mm:35: Can you synthesize the property here? @synthesize matches = _matches; https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... ios/chrome/browser/voice/speech_input_locale_match_config.mm:57: } This will also be handled by property synthesis https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... ios/chrome/browser/voice/speech_input_locale_match_config.mm:87: @end This whole @interface section can be removed if you synthesize the properties. https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... ios/chrome/browser/voice/speech_input_locale_match_config.mm:90: @synthesize matchedLocaleCode = _matchedLocaleCode; @synthesize matchingLocaleCodes = _matchingLocaleCodes; @synthesize matchingLanguages = _matchingLanguages; https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... ios/chrome/browser/voice/speech_input_locale_match_config.mm:112: } All three of these can be removed.
https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... File ios/chrome/browser/voice/speech_input_locale_match_config.mm (right): https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... ios/chrome/browser/voice/speech_input_locale_match_config.mm:27: } On 2017/05/25 19:07:36, kkhorimoto_ wrote: > This whole block within the curly braces can be removed since we can just > synthesize the variable now. Done. https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... ios/chrome/browser/voice/speech_input_locale_match_config.mm:35: On 2017/05/25 19:07:36, kkhorimoto_ wrote: > Can you synthesize the property here? > > @synthesize matches = _matches; Done. https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... ios/chrome/browser/voice/speech_input_locale_match_config.mm:57: } On 2017/05/25 19:07:36, kkhorimoto_ wrote: > This will also be handled by property synthesis Ok, so I just deleted the line 56, please let me know if more is needed. https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... ios/chrome/browser/voice/speech_input_locale_match_config.mm:87: @end On 2017/05/25 19:07:36, kkhorimoto_ wrote: > This whole @interface section can be removed if you synthesize the properties. Done. https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... ios/chrome/browser/voice/speech_input_locale_match_config.mm:90: On 2017/05/25 19:07:37, kkhorimoto_ wrote: > @synthesize matchedLocaleCode = _matchedLocaleCode; > @synthesize matchingLocaleCodes = _matchingLocaleCodes; > @synthesize matchingLanguages = _matchingLanguages; Done. https://codereview.chromium.org/2894623004/diff/1/ios/chrome/browser/voice/sp... ios/chrome/browser/voice/speech_input_locale_match_config.mm:112: } On 2017/05/25 19:07:37, kkhorimoto_ wrote: > All three of these can be removed. Done.
The CQ bit was checked by lindsayw@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm https://codereview.chromium.org/2894623004/diff/20001/ios/chrome/browser/voic... File ios/chrome/browser/voice/speech_input_locale_match_config.mm (right): https://codereview.chromium.org/2894623004/diff/20001/ios/chrome/browser/voic... ios/chrome/browser/voice/speech_input_locale_match_config.mm:25: } Can you also remove the curly braces themselves? They're not necessary if you aren't declaring any ivars.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2894623004/diff/20001/ios/chrome/browser/voic... File ios/chrome/browser/voice/speech_input_locale_match_config.mm (right): https://codereview.chromium.org/2894623004/diff/20001/ios/chrome/browser/voic... ios/chrome/browser/voice/speech_input_locale_match_config.mm:25: } On 2017/06/01 19:25:27, kkhorimoto_ wrote: > Can you also remove the curly braces themselves? They're not necessary if you > aren't declaring any ivars. Done.
The CQ bit was checked by lindsayw@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kkhorimoto@chromium.org Link to the patchset: https://codereview.chromium.org/2894623004/#ps40001 (title: "Removed curly braces.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by lindsayw@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496427146776230,
"parent_rev": "b6e12e26514866f3e92bfed8725a3338f0e4409c", "commit_rev":
"a60bca2c4ac7f84008c5345db7ba08561a0eb6bc"}
Message was sent while issue was closed.
Description was changed from ========== [ObjC ARC] Converts ios/chrome/browser/voice:voice to ARC. Automatically generated ARCMigrate commit Notable issues:None BUG=624363 TEST=None ========== to ========== [ObjC ARC] Converts ios/chrome/browser/voice:voice to ARC. Automatically generated ARCMigrate commit Notable issues:None BUG=624363 TEST=None Review-Url: https://codereview.chromium.org/2894623004 Cr-Commit-Position: refs/heads/master@{#476745} Committed: https://chromium.googlesource.com/chromium/src/+/a60bca2c4ac7f84008c5345db7ba... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/a60bca2c4ac7f84008c5345db7ba... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
