Skip to content

Conversation

@sethladd
Copy link
Contributor

Fixes #10696

@sethladd
Copy link
Contributor Author

cc @Hixie @devoncarew

// default as custom dimension 1 (configured in our analytics account).
_analytics.setSessionValue('dimension1', os.name);

// Report the branch (or, "channel") name.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove this comment (it doesn't really add anything you can't figure out from the code), remove the blank line before it, and then update the earlier comment to cover both cases.

@devoncarew
Copy link
Contributor

lgtm w/ Hixie's suggestion

@sethladd
Copy link
Contributor Author

@devoncarew @Hixie ptal thanks!

// found in the LICENSE file.

import 'dart:async';
import 'dart:io';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this import used?

Copy link
Contributor

@devoncarew devoncarew Jun 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I don't believe we generally want to import dart:io in flutter_tools)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devoncarew
Copy link
Contributor

lgtm w/ the removed import

@sethladd
Copy link
Contributor Author

Ah, thanks! That was left over from debugging.

@sethladd sethladd merged commit b471a9c into flutter:master Jun 20, 2017
@sethladd sethladd deleted the double-flx branch June 20, 2017 15:21
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jul 1, 2017
* send channel name as a custom dimension

* tweaks from review

* remove unused import
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Request for analytics: clearly send the "channel" our users have installed

4 participants