Update mig.py to take an explicit sdk argument.

Update mig.py to take an explicit sdk argument.

BUG=690734
Review-Url: https://codereview.chromium.org/2685233002
Cr-Commit-Position: refs/heads/master@{#449550}
Committed: https://chromium.googlesource.com/chromium/src/+/53f2146935506b4f382705b605dffec41b5519eb

[erikchen, 2017-02-10 01:56:02]

erikchen@chromium.org changed reviewers:
+ mark@chromium.org

[erikchen, 2017-02-10 01:56:03]

mark: Please review.

I addressed all your comments from the polygerrit review except for passing args
to parse_args(), which by default already pulls from sys.argv.

This CL changes the interface to mig.py, so it seems better to make the
corresponding GN changes along with the CL, and then upstream back to crashpad.

[Mark Mentovai, 2017-02-10 02:11:39]

LGTM % comment

https://codereview.chromium.org/2685233002/diff/1/third_party/crashpad/crashp...
File third_party/crashpad/crashpad/util/mach/mig.py (right):

https://codereview.chromium.org/2685233002/diff/1/third_party/crashpad/crashp...
third_party/crashpad/crashpad/util/mach/mig.py:119: parsed = parser.parse_args()
OK.

Either put “args” here OR get rid of the “args” argument to main(), then. My
preference is the former (and I bet you’ll agree after you read the rest), but
if you really are opposed to it, it’s not the worst thing. You should at least
clean up after yourself and get rid of the unused argument if it is in fact
unused.

The reason that I prefer the “args” argument is so that you can use this as a
library. That way, some other code can import mig and call mig.main(arguments).
If you just use parse_args(), it always pulls from sys.argv, which makes this
impossible. Incidentally, this is also why we always put main into its own
function and call it wrapped in the __name__ check.

[erikchen, 2017-02-10 03:24:19]

https://codereview.chromium.org/2685233002/diff/1/third_party/crashpad/crashp...
File third_party/crashpad/crashpad/util/mach/mig.py (right):

https://codereview.chromium.org/2685233002/diff/1/third_party/crashpad/crashp...
third_party/crashpad/crashpad/util/mach/mig.py:119: parsed = parser.parse_args()
On 2017/02/10 02:11:39, Mark Mentovai wrote:
> OK.
> 
> Either put “args” here OR get rid of the “args” argument to main(), then. My
> preference is the former (and I bet you’ll agree after you read the rest), but
> if you really are opposed to it, it’s not the worst thing. You should at least
> clean up after yourself and get rid of the unused argument if it is in fact
> unused.
> 
> The reason that I prefer the “args” argument is so that you can use this as a
> library. That way, some other code can import mig and call
mig.main(arguments).
> If you just use parse_args(), it always pulls from sys.argv, which makes this
> impossible. Incidentally, this is also why we always put main into its own
> function and call it wrapped in the __name__ check.

Done

[erikchen, 2017-02-10 03:24:32]

The CQ bit was checked by erikchen@chromium.org

[erikchen, 2017-02-10 03:24:32]

The patchset sent to the CQ was uploaded after l-g-t-m from mark@chromium.org
Link to the patchset: https://codereview.chromium.org/2685233002/#ps20001
(title: "Comments from mark.")

[commit-bot: I haz the power, 2017-02-10 03:24:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-10 05:35:54]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486697072228520,
"parent_rev": "f05f8abb0c65ea13a76fcf6b49cff9c12eb820e0", "commit_rev":
"53f2146935506b4f382705b605dffec41b5519eb"}

[commit-bot: I haz the power, 2017-02-10 05:36:35]

Description was changed from

==========
Update mig.py to take an explicit sdk argument.

BUG=690734
==========

to

==========
Update mig.py to take an explicit sdk argument.

BUG=690734

Review-Url: https://codereview.chromium.org/2685233002
Cr-Commit-Position: refs/heads/master@{#449550}
Committed:
https://chromium.googlesource.com/chromium/src/+/53f2146935506b4f382705b605df...
==========

[commit-bot: I haz the power, 2017-02-10 05:36:35]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/53f2146935506b4f382705b605df...