Fix recognition of "linuxchromeos" directory in cr

Fix recognition of "linuxchromeos" directory in cr

Since the strings "linuxchromeos" and "linux" both match the
directory name, cr throws an error for not being able to find the
platform. This CL fixes this issue to match substrings better.

Committed: https://crrev.com/6dd04c213d6c79b2bc0cd54078e548cc64611dd7
Cr-Commit-Position: refs/heads/master@{#368115}

[ssid, 2016-01-06 17:46:22]

ssid@chromium.org changed reviewers:
+ petrcermak@chromium.org

[ssid, 2016-01-06 17:46:22]

PTAL, Thanks.

[petrcermak, 2016-01-06 18:26:41]

LGTM with 2 inline comments and some description nits:

* Please change the title of the patch to what the patch does (rather than the
current state of affairs), e.g. "Fix recognition of 'linuxchromeos' directory in
cr".
* s/the string/the strings/
* s/match to/match/
* Either provide a bug ID, or remove the "BUG=" line.

Thanks,
Petr

https://codereview.chromium.org/1562083002/diff/20001/tools/cr/cr/commands/in...
File tools/cr/cr/commands/init.py (right):

https://codereview.chromium.org/1562083002/diff/20001/tools/cr/cr/commands/in...
tools/cr/cr/commands/init.py:86: # Get longest matching string and check if
others are substrings. This
nit: "the longest" and "the others"

https://codereview.chromium.org/1562083002/diff/20001/tools/cr/cr/commands/in...
tools/cr/cr/commands/init.py:89: matches_are_substrings = set(p in platform for
p in matches)
Please change this to the following to make it more readable (plus it removes
the unnecessary construction of a set):

all_matches_are_substrings = all(p in platform for p in matches)
if all_matches_are_substrings or not matches:
  ...

[ssid, 2016-01-07 17:48:44]

Description was changed from

==========
CR does not recognize "linuxchromeos" directory

Since the string "linuxchromeos" and "linux" both match to the
directory name, cr throws an error for not being able to find the
platform. This CL fixes this issue to match substrings better.

BUG=
==========

to

==========
Fix recognition of "linuxchromeos" directory in cr

Since the strings "linuxchromeos" and "linux" both match the
directory name, cr throws an error for not being able to find the
platform. This CL fixes this issue to match substrings better.
==========

[ssid, 2016-01-07 17:50:01]

The CQ bit was checked by ssid@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-07 17:50:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1562083002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1562083002/40001

[ssid, 2016-01-07 17:50:27]

Thanks, fixed.

https://codereview.chromium.org/1562083002/diff/20001/tools/cr/cr/commands/in...
File tools/cr/cr/commands/init.py (right):

https://codereview.chromium.org/1562083002/diff/20001/tools/cr/cr/commands/in...
tools/cr/cr/commands/init.py:86: # Get longest matching string and check if
others are substrings. This
On 2016/01/06 18:26:41, petrcermak wrote:
> nit: "the longest" and "the others"

Done.

https://codereview.chromium.org/1562083002/diff/20001/tools/cr/cr/commands/in...
tools/cr/cr/commands/init.py:89: matches_are_substrings = set(p in platform for
p in matches)
On 2016/01/06 18:26:41, petrcermak wrote:
> Please change this to the following to make it more readable (plus it removes
> the unnecessary construction of a set):
> 
> all_matches_are_substrings = all(p in platform for p in matches)
> if all_matches_are_substrings or not matches:
>   ...

Done.

[petrcermak, 2016-01-07 17:56:16]

Looks perfect to me!

Thanks,
Petr

[commit-bot: I haz the power, 2016-01-07 18:01:15]

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

[commit-bot: I haz the power, 2016-01-07 18:01:16]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ssid, 2016-01-07 18:34:19]

The CQ bit was checked by ssid@chromium.org

[ssid, 2016-01-07 18:34:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
petrcermak@chromium.org
Link to the patchset: https://codereview.chromium.org/1562083002/#ps60001
(title: "Fix rebase.")

[commit-bot: I haz the power, 2016-01-07 18:35:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1562083002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1562083002/60001

[commit-bot: I haz the power, 2016-01-07 18:59:21]

Description was changed from

==========
Fix recognition of "linuxchromeos" directory in cr

Since the strings "linuxchromeos" and "linux" both match the
directory name, cr throws an error for not being able to find the
platform. This CL fixes this issue to match substrings better.
==========

to

==========
Fix recognition of "linuxchromeos" directory in cr

Since the strings "linuxchromeos" and "linux" both match the
directory name, cr throws an error for not being able to find the
platform. This CL fixes this issue to match substrings better.
==========

[commit-bot: I haz the power, 2016-01-07 18:59:22]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-07 19:00:07]

Description was changed from

==========
Fix recognition of "linuxchromeos" directory in cr

Since the strings "linuxchromeos" and "linux" both match the
directory name, cr throws an error for not being able to find the
platform. This CL fixes this issue to match substrings better.
==========

to

==========
Fix recognition of "linuxchromeos" directory in cr

Since the strings "linuxchromeos" and "linux" both match the
directory name, cr throws an error for not being able to find the
platform. This CL fixes this issue to match substrings better.

Committed: https://crrev.com/6dd04c213d6c79b2bc0cd54078e548cc64611dd7
Cr-Commit-Position: refs/heads/master@{#368115}
==========

[commit-bot: I haz the power, 2016-01-07 19:00:07]

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

[alex clarke (OOO till 29th), 2016-01-08 11:08:05]

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

[alex clarke (OOO till 29th), 2016-01-08 11:08:06]

This patch breaks:  cr init -o=out_android/Release

Please fix or revert.

[ssid, 2016-01-08 11:27:43]

On 2016/01/08 11:08:06, alexclarke1 wrote:
> This patch breaks:  cr init -o=out_android/Release
> 
> Please fix or revert.

Thanks for the comment. Fixed it here
https://codereview.chromium.org/1566393002/

[alex clarke (OOO till 29th), 2016-01-08 11:44:05]

On 2016/01/08 11:27:43, ssid wrote:
> On 2016/01/08 11:08:06, alexclarke1 wrote:
> > This patch breaks:  cr init -o=out_android/Release
> > 
> > Please fix or revert.
> 
> Thanks for the comment. Fixed it here
> https://codereview.chromium.org/1566393002/

Thanks for the fix!