Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse telemetry ini config as boolean and make telemetry opt-in. #553

Merged
merged 1 commit into from
Jan 5, 2023

Conversation

jshort
Copy link
Collaborator

@jshort jshort commented Dec 13, 2022

In issue #504, it was brought to attention that while the command line arg for enabling telemetry was boolean, the ini config in et.cfg needed to be an int to change the value. SimpleIni has a GetBoolValue function that treats the strings "0", "off", "no", "false" as boolean false and "1", "on", "true" as boolean true.

Also, this will make telemetry opt-in as opposed to opt-out as has been requested by multiple users. The supplied et.cfg file will still have telemetry on.

In issue MisterTea#504, it was brought to attention that while the command line
arg for enabling telemetry was boolean, the ini config in et.cfg needed
to be an int to change the value.  SimpleIni has a GetBoolValue function
that treats the strings "0", "off", "no", "false" as boolean false and
"1", "on", "true" as boolean true.

Also, this will make telemetry opt-in as opposed to opt-out as has been
requested by multiple users.  The supplied et.cfg file will still have
telemetry on.
@codecov-commenter
Copy link

Codecov Report

Base: 70.75% // Head: 71.21% // Increases project coverage by +0.45% 🎉

Coverage data is based on head (22d970e) compared to base (91099f6).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #553      +/-   ##
==========================================
+ Coverage   70.75%   71.21%   +0.45%     
==========================================
  Files          49       49              
  Lines        3074     3074              
==========================================
+ Hits         2175     2189      +14     
+ Misses        899      885      -14     
Impacted Files Coverage Δ
src/base/ServerClientConnection.cpp 92.85% <0.00%> (-3.58%) ⬇️
src/base/Connection.cpp 87.82% <0.00%> (-0.87%) ⬇️
test/ConnectionTest.cpp 94.44% <0.00%> (-0.51%) ⬇️
src/base/SocketHandler.cpp 63.63% <0.00%> (+1.29%) ⬆️
src/terminal/UserJumphostHandler.cpp 64.80% <0.00%> (+3.19%) ⬆️
src/base/ClientConnection.cpp 77.27% <0.00%> (+3.63%) ⬆️
src/base/ServerConnection.cpp 70.75% <0.00%> (+7.54%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jshort
Copy link
Collaborator Author

jshort commented Jan 5, 2023

I'd say this has baked long enough without disagreement, merging...

@jshort jshort merged commit 7289e04 into MisterTea:master Jan 5, 2023
jshort added a commit to jshort/EternalTerminal that referenced this pull request May 2, 2024
In addition to opt-in for telemetry (see MisterTea#553) this uses the log
directory override from MisterTea#556 to place the sentry/telemetry logs.
jshort added a commit to jshort/EternalTerminal that referenced this pull request May 2, 2024
In addition to opt-in for telemetry (see MisterTea#553) this uses the log
directory override from MisterTea#556 to place the sentry/telemetry logs.
MisterTea pushed a commit that referenced this pull request May 2, 2024
In addition to opt-in for telemetry (see #553) this uses the log
directory override from #556 to place the sentry/telemetry logs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants