isolate: pull exparchive and archive into functions with similar signatures.

isolate: pull exparchive and archive into functions with similar signatures.

BUG=692940
Review-Url: https://codereview.chromium.org/2989173002
Committed: https://github.com/luci/luci-go/commit/258f0c01c308e369ef4d9c96cfddfb25e7944ee9

[mcgreevy, 2017-08-01 07:25:52]

mcgreevy@chromium.org changed reviewers:
+ tansell@chromium.org

[mithro, 2017-08-02 04:28:34]

Generally LGTM.

Some minor comments.

https://codereview.chromium.org/2989173002/diff/1/client/cmd/isolate/archive.go
File client/cmd/isolate/archive.go (right):

https://codereview.chromium.org/2989173002/diff/1/client/cmd/isolate/archive....
client/cmd/isolate/archive.go:101: func doArchive(ctx context.Context, client
*isolatedclient.Client, archiveOpts *isolate.ArchiveOptions, quiet bool, start
time.Time) (*logpb.IsolateClientEvent_ArchiveDetails, error) {
It seems a bit weird to pass in start time here, but I can't think of a better
option....

https://codereview.chromium.org/2989173002/diff/1/client/cmd/isolate/exp_arch...
File client/cmd/isolate/exp_archive.go (right):

https://codereview.chromium.org/2989173002/diff/1/client/cmd/isolate/exp_arch...
client/cmd/isolate/exp_archive.go:78: func (c *expArchiveRun) main() error {
The main() in cmdExpArchive and cmdArchive should look almost identical now,
right?

[mcgreevy, 2017-08-02 05:16:38]

https://codereview.chromium.org/2989173002/diff/1/client/cmd/isolate/archive.go
File client/cmd/isolate/archive.go (right):

https://codereview.chromium.org/2989173002/diff/1/client/cmd/isolate/archive....
client/cmd/isolate/archive.go:101: func doArchive(ctx context.Context, client
*isolatedclient.Client, archiveOpts *isolate.ArchiveOptions, quiet bool, start
time.Time) (*logpb.IsolateClientEvent_ArchiveDetails, error) {
On 2017/08/02 04:28:34, mithro wrote:
> It seems a bit weird to pass in start time here, but I can't think of a better
> option....

Yeah, I hesitated to do so, but didn't want to change the logging right now, and
other options I can think of seemed just as bad.

https://codereview.chromium.org/2989173002/diff/1/client/cmd/isolate/exp_arch...
File client/cmd/isolate/exp_archive.go (right):

https://codereview.chromium.org/2989173002/diff/1/client/cmd/isolate/exp_arch...
client/cmd/isolate/exp_archive.go:78: func (c *expArchiveRun) main() error {
On 2017/08/02 04:28:34, mithro wrote:
> The main() in cmdExpArchive and cmdArchive should look almost identical now,
> right?

Yes.

[mcgreevy, 2017-08-03 06:03:25]

The CQ bit was checked by mcgreevy@chromium.org

[commit-bot: I haz the power, 2017-08-03 06:03:36]

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-08-03 06:07:24]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1501740205849760, "parent_rev":
"566e3edf53ab30546bc280d6d4e6b188414f2325", "commit_rev":
"258f0c01c308e369ef4d9c96cfddfb25e7944ee9"}

[commit-bot: I haz the power, 2017-08-03 06:07:27]

Description was changed from

==========
isolate: pull exparchive and archive into functions with similar signatures.

BUG=692940
==========

to

==========
isolate: pull exparchive and archive into functions with similar signatures.

BUG=692940

Review-Url: https://codereview.chromium.org/2989173002
Committed:
https://github.com/luci/luci-go/commit/258f0c01c308e369ef4d9c96cfddfb25e7944ee9
==========

[commit-bot: I haz the power, 2017-08-03 06:07:28]

Committed patchset #1 (id:1) as
https://github.com/luci/luci-go/commit/258f0c01c308e369ef4d9c96cfddfb25e7944ee9