8000 Updater for websockets by basicer · Pull Request #3581 · Cockatrice/Cockatrice · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Updater for websockets #3581

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 19, 2019
Merged

Updater for websockets #3581

merged 2 commits into from
Feb 19, 2019

Conversation

basicer
Copy link
Member
@basicer basicer commented Feb 17, 2019

Related Ticket(s)

Short roundup of the initial problem

  • We need to force update to get users to switch to the websocket port

What will change with this Pull Request?

  • The server updater won't erase passwords
  • The server updater is triggered on client update.

@basicer basicer force-pushed the updater-for-websockets branch from 3067170 to 961d332 Compare February 17, 2019 23:51
@basicer basicer force-pushed the updater-for-websockets branch from 961d332 to eedb46f Compare February 18, 2019 11:43
tooomm
tooomm approved these changes Feb 18, 2019
View reviewed changes
Copy link
Contributor
@tooomm tooomm left a comment

Choose a reason for hiding this comment

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

Seems to work as intended in the client.

(Code review pending)

ebbit1q
ebbit1q reviewed Feb 19, 2019
View reviewed changes
cockatrice/src/settings/serverssettings.cpp

for (int i = 0; i < size; i++) {
if (serv == getValue(QString("server%1").arg(i), "server", "server_details").toString()) {
setValue(port, QString("port%1").arg(i), "server", "server_details");
Copy link
Member

Choose a reason for hiding this comment

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

I understand why this is copied but being inconsistent on one function having the port be optional while the other doesn't just irks me.

@basicer basicer merged commit d519f99 into Cockatrice:master Feb 19, 2019
@ctrlaltca ctrlaltca mentioned this pull request Feb 19, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebbit1q ebbit1q ebbit1q left review comments

@ctrlaltca ctrlaltca ctrlaltca approved these changes

@tooomm tooomm tooomm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@basicer @ctrlaltca @tooomm @ebbit1q
0