ui: Add another presubmit check to catch two more scoped_ptr usages.

ui: Add another presubmit check to catch two more scoped_ptr usages.

This should catch the following usages:

1- return scoped_ptr<T>(foo)
2- bar = scoped_ptr<T>(foo)

And recommend the solo usage of make_scoped_ptr().

The entries were found with the following command line:

$ git grep -E '(=|\breturn)\s*scoped_ptr<.*?>([^)]+)'

BUG=None
TEST=g cl presubmit -uv
R=sky@chromium.org,maruel@chromium.org

Committed: https://crrev.com/63f25bef305bd8143766e2e1a98bf01548c78376
Cr-Commit-Position: refs/heads/master@{#317669}

[tfarina, 2015-02-13 01:42:06]

Marc-Antoine, could you review this?

Scott, for OWNERS approval after Marc-Antoine is happy with this.

This is for CL https://codereview.chromium.org/922923002/.

[Marc-Antoine Ruel (Google), 2015-02-13 02:16:51]

maruel@google.com changed reviewers:
+ maruel@google.com

[Marc-Antoine Ruel (Google), 2015-02-13 02:16:51]

https://codereview.chromium.org/919253002/diff/20001/ui/PRESUBMIT.py
File ui/PRESUBMIT.py (right):

https://codereview.chromium.org/919253002/diff/20001/ui/PRESUBMIT.py#newcode11
ui/PRESUBMIT.py:11: import re
not needed

https://codereview.chromium.org/919253002/diff/20001/ui/PRESUBMIT.py#newcode32
ui/PRESUBMIT.py:32: if
re.search(r'(=|\breturn)\s*scoped_ptr<.*?(?<!])>\([^)]+\)', line):
input_api.re. ...

could be simplified to
... scoped_ptr<[^\[\]>]+> ...

but is this check really valuable?

[tfarina, 2015-02-13 02:28:40]

PTAL

https://codereview.chromium.org/919253002/diff/20001/ui/PRESUBMIT.py
File ui/PRESUBMIT.py (right):

https://codereview.chromium.org/919253002/diff/20001/ui/PRESUBMIT.py#newcode32
ui/PRESUBMIT.py:32: if
re.search(r'(=|\breturn)\s*scoped_ptr<.*?(?<!])>\([^)]+\)', line):
On 2015/02/13 02:16:51, Marc-Antoine Ruel (Google) wrote:
> input_api.re. ...
> 
> could be simplified to
> ... scoped_ptr<[^\[\]>]+> ...
> 
Done.

> but is this check really valuable?
Yep, I would like to have the same checks of //cc/.

[Marc-Antoine Ruel (Google), 2015-02-13 02:31:48]

lgtm, assuming there's no false positive

https://codereview.chromium.org/919253002/diff/60001/ui/PRESUBMIT.py
File ui/PRESUBMIT.py (right):

https://codereview.chromium.org/919253002/diff/60001/ui/PRESUBMIT.py#newcode12
ui/PRESUBMIT.py:12: r'.*\.(cc|h|mm)$',
I like +2. :)

I'd recommend not touching it just to keep the history intact.

[tfarina, 2015-02-13 02:35:17]

New patchsets have been uploaded after l-g-t-m from maruel@google.com

[tfarina, 2015-02-13 02:37:05]

Done.

https://codereview.chromium.org/919253002/diff/60001/ui/PRESUBMIT.py
File ui/PRESUBMIT.py (right):

https://codereview.chromium.org/919253002/diff/60001/ui/PRESUBMIT.py#newcode12
ui/PRESUBMIT.py:12: r'.*\.(cc|h|mm)$',
On 2015/02/13 02:31:48, Marc-Antoine Ruel (Google) wrote:
> I like +2. :)
> 
> I'd recommend not touching it just to keep the history intact.

That was format-webkitpy --chromium. Ick :(

I also like 2 spaces and to keep the history intact as much as possible. 

So I reverted this change.

[tfarina, 2015-02-20 11:03:57]

tfarina@chromium.org changed reviewers:
+ sadrul@chromium.org

[tfarina, 2015-02-20 11:03:58]

Sadrul, could you approve this as well, now that dependent change was approved
and landed in https://codereview.chromium.org/922923002/?

[sky, 2015-02-23 15:50:05]

LGTM

[tfarina, 2015-02-23 16:52:28]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2015-02-23 16:53:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/919253002/80001

[commit-bot: I haz the power, 2015-02-23 16:56:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-23 16:56:01]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[tfarina, 2015-02-23 21:36:39]

The CQ bit was checked by tfarina@chromium.org

[tfarina, 2015-02-23 21:36:39]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
maruel@google.com
Link to the patchset: https://codereview.chromium.org/919253002/#ps100001
(title: "REBASE")

[commit-bot: I haz the power, 2015-02-23 21:37:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/919253002/100001

[commit-bot: I haz the power, 2015-02-23 22:42:46]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-02-23 22:43:47]

Patchset 6 (id:??) landed as
https://crrev.com/63f25bef305bd8143766e2e1a98bf01548c78376
Cr-Commit-Position: refs/heads/master@{#317669}

[scottmg, 2015-02-24 00:18:32]

scottmg@chromium.org changed reviewers:
+ scottmg@chromium.org

[scottmg, 2015-02-24 00:18:33]

https://codereview.chromium.org/919253002/diff/100001/ui/PRESUBMIT.py
File ui/PRESUBMIT.py (right):

https://codereview.chromium.org/919253002/diff/100001/ui/PRESUBMIT.py#newcode38
ui/PRESUBMIT.py:38: if re.search(r'\bscoped_ptr<.*?>\(\)', line):
i think this should be input_api.re? or there needs to be an import re anyway.

i'm going to revert, as it's blocking upload in ui/.

[scottmg, 2015-02-24 00:19:09]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/954473003/ by scottmg@chromium.org.

The reason for reverting is: Presubmit in ui/ failing with

...
File "<string>", line 38, in CheckScopedPtr
NameError: global name 're' is not defined.

[tfarina, 2015-02-24 00:23:17]

I thinking this Scott

On Mon, Feb 23, 2015 at 9:19 PM, <scottmg@chromium.org> wrote:

> A revert of this CL (patchset #6 id:100001) has been created in
> https://codereview.chromium.org/954473003/ by scottmg@chromium.org.
>
> The reason for reverting is: Presubmit in ui/ failing with
>
> ...
> File "<string>", line 38, in CheckScopedPtr
> NameError: global name 're' is not defined.
>
> https://codereview.chromium.org/919253002/
>



-- 
Thiago Farina

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[tfarina, 2015-02-24 00:23:26]

On Mon, Feb 23, 2015 at 9:23 PM, Thiago Farina <tfarina@chromium.org> wrote:

> I thinking this Scott
>
> fixing

> On Mon, Feb 23, 2015 at 9:19 PM, <scottmg@chromium.org> wrote:
>
>> A revert of this CL (patchset #6 id:100001) has been created in
>> https://codereview.chromium.org/954473003/ by scottmg@chromium.org.
>>
>> The reason for reverting is: Presubmit in ui/ failing with
>>
>> ...
>> File "<string>", line 38, in CheckScopedPtr
>> NameError: global name 're' is not defined.
>>
>> https://codereview.chromium.org/919253002/
>>
>
>
>
> --
> Thiago Farina
>



-- 
Thiago Farina

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.