Seanmcm/telemetry fix#1494
Conversation
| previousCppSettings[key] = val; | ||
| result[key] = (key === "clang_format_path") ? "..." : String(previousCppSettings[key]); | ||
| switch (key.toLowerCase()) { | ||
| case "clang_format_path": { |
There was a problem hiding this comment.
I think we should just remove clang_format_path from telemetry
There was a problem hiding this comment.
perhaps the above if-expression should be if (filter(key, val, settings) && isTelemetryAllowed(key, val)) and then move all of the special cases to a new function isTelemetryAllowed and filter them there so that this function can remain simple.
There was a problem hiding this comment.
What do you mean by "remove clang_format_path from telemetry"? Do you mean just use "..." like we were doing before?
There was a problem hiding this comment.
I meant to just filter it out completely (never send that property)
There was a problem hiding this comment.
Okay, I stopped sending it.
| break; | ||
| } | ||
| default: { | ||
| if (val.startsWith("{") && val.endsWith("}")) { |
There was a problem hiding this comment.
is this distinction necessary? can we just send "..." for anything that doesn't match one of the enum values?
| switch (key.toLowerCase()) { | ||
| case "clang_format_path": { | ||
| if (val) { | ||
| switch (val.toLowerCase()) { |
There was a problem hiding this comment.
val is any, so I don't think you can safely call this without converting to string first.
| case "clang_format_style": | ||
| case "clang_format_fallbackstyle": { | ||
| if (val) { | ||
| switch (val.toLowerCase()) { |
There was a problem hiding this comment.
same comment as above. val must be confirmed to be a string before this can be called on it.
| case "webkit": | ||
| case "file": | ||
| case "none": { | ||
| result[key] = String(previousCppSettings[key]); |
There was a problem hiding this comment.
Remember also that we have to rename the keys since we are making a change to what is logged for these settings.
There was a problem hiding this comment.
Oh, yeah, I forgot.
There was a problem hiding this comment.
I added a 2 at the end.
2 participants
No description provided.