Collect anonymous metrics #227
Conversation
Also add a setting for opting out of usage data
|
@haacked I've added the options page to save the user settings, with a |
|
Why don't we split this PR into two. Add the options whether we're collecting data or not. Then a separate PR for the metrics. Thoughts? |
| <DockPanel> | ||
| <WrapPanel DockPanel.Dock="Top"> | ||
| <CheckBox x:Name="chkMetrics" VerticalAlignment="Center" /> | ||
| <Label Content="{x:Static prop:Resources.Options_MetricsLabel}" /> |
There was a problem hiding this comment.
We should probably add a hyperlink here that points to https://visualstudio.github.com/samples/. That page shows examples of the type of data we collect.
Here's the Desktop example. Obviously we'll need to add that page to the Visual Studio website, but lets add the link now. 😄
There was a problem hiding this comment.
Turns out that we already have a samples page! But it looked terrible. I fixed it up with some placeholder content. Please add a link to https://visualstudio.github.com/samples.html
Thanks!
|
@shana a few code review comments for you. I'll look into the other checkboxes. |
Sounds reasonable. Should I add an issue to track these tasks instead so we don't lose the overall picture? |
Seems legit to me. |
|
Part 1 and 2 of #229 |
|
So instead of all this manual checking to make sure things are synchronized, I've switched the code to automatically generate the interface and the implementation of the settings class based on a json file that lists the settings we want with their types and default values. This way we only have to change things in the json file. VS regenerates the files on build if the json file is changed. |
f6ae3ca to
1aa3dce
Compare
|
I also moved some other settings files that were spread around (colors, constants, etc) to Settings folder along with this stuff, to make things easier to find. |
|
⚽ |
| <# | ||
| foreach (var j in json["settings"].Children()) { | ||
| #> | ||
| <#= j["type"] #> <#= j["name"] #> { get; set; } |
There was a problem hiding this comment.
I fucking love this approach! Great use of text transformation templates!
I love it so much, I'm going to nitpick here and suggest that should be indented four spaces not eight. 😛
There was a problem hiding this comment.
But but but but... then the properties will be indented at the level of the interface definition!
There was a problem hiding this comment.
Don't worry about it. 😄 Trying to get generated code formatted correctly with TT is a time sink.
|
This is great! Had some nitpicky comments about spacing which aren't really important. The main question is whether we need to commit the generated files or not. |
|
I just validated the build on a fresh clone, things are generated correctly and it only rebuilds if files are changed, so 👍 |
|
We should probably add the generated files to Sorry if I'm being nitpicky, but I want this to be a template for how I do this in more places. 😄 |
|
Wonderful! |
This PR adds a settings service and options page to handle user settings. Also adds one setting: anonymous usage data collection opt-out.