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

Mumble: remove expert mode. #2922

Merged
merged 2 commits into from Mar 16, 2017

Conversation

@mkrautz
Copy link
Member

commented Mar 11, 2017

This commmit removes the concept of expert mode configuration
from the Mumble client.

@mkrautz mkrautz requested review from Kissaki, hacst and davidebeatrici Mar 11, 2017

@davidebeatrici davidebeatrici added the ui label Mar 11, 2017

@Kissaki

This comment has been minimized.

Copy link
Member

commented Mar 13, 2017

Typo in commit message "commmit"

@Kissaki
Copy link
Member

left a comment

Did you deliberately not update the layout row value because it continues to work even with "empty" layout cells?

Now that we removed a setting, I wonder if we should keep a list of removed settings in Settings.h, so we don't accidentally reuse them. Probably. A typed check would be even better ofc.

@mkrautz mkrautz force-pushed the mkrautz:remove-expert-mode branch from 9b88f26 to 3fc491c Mar 16, 2017

mkrautz added 2 commits Mar 16, 2017
Mumble: remove expert mode.
This commit removes the concept of expert mode configuration
from the Mumble client.
Settings: document that expert mode has been removed.
We keep the setting around, to avoid re-using it in the future.

@mkrautz mkrautz force-pushed the mkrautz:remove-expert-mode branch from 3fc491c to 0d2e891 Mar 16, 2017

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Mar 16, 2017

@Kissaki PTAL.

@Kissaki
Copy link
Member

left a comment

Did you also keep the load+save of the setting?

I didn't check for completeness of removal.

LGTM.

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Mar 16, 2017

Yes, I kept save/load.

@mkrautz mkrautz merged commit b16983f into mumble-voip:master Mar 16, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.