Support 0.31 SDK, vrooom #2277

2019-02-11T23:57:15+01:00

tzarebczan commented

2019-02-11 23:57:15 +01:00

(Migrated from github.com)

Fixes https://github.com/lbryio/lbry-desktop/issues/2231

neb-b (Migrated from github.com) reviewed 2019-02-12 00:39:50 +01:00

src/renderer/page/file/view.jsx

neb-b (Migrated from github.com) commented

2019-02-12 00:39:50 +01:00

Can we just remove fileInfo so we don't need this?

Can we just remove `fileInfo` so we don't need this?

neb-b (Migrated from github.com) reviewed 2019-02-12 00:45:29 +01:00

src/renderer/page/settings/view.jsx

neb-b (Migrated from github.com) commented

2019-02-12 00:45:29 +01:00

We shouldn't keep this as a local setting. It should stay in the daemon only. Then we don't have the same setting being tracked in two places.

neb-b (Migrated from github.com) requested changes 2019-02-12 00:46:22 +01:00

neb-b (Migrated from github.com) left a comment

Looks ok. The only thing to change is to remove the localSetting stuff that was added for these values. If we need to add something so we can set better defaults, we can do that, but the daemon should remain the single source of truth.

Looks ok. The only thing to change is to remove the `localSetting` stuff that was added for these values. If we need to add something so we can set better defaults, we can do that, but the daemon should remain the single source of truth.

neb-b commented

2019-02-12 00:50:42 +01:00

(Migrated from github.com)

To add defaults, we can add something here:
https://github.com/lbryio/lbry-desktop/blob/master/src/renderer/redux/reducers/settings.js#L47

daemonSettings: {
  [DAEMON_SETTINGS.MAX_KEY_FEE]: ...,
}

It would be nice if they are constants, but not a huge deal right now. We can open an issue for that with help wanted maybe.

To add defaults, we can add something here: https://github.com/lbryio/lbry-desktop/blob/master/src/renderer/redux/reducers/settings.js#L47 ``` daemonSettings: { [DAEMON_SETTINGS.MAX_KEY_FEE]: ..., } ``` It would be nice if they are constants, but not a huge deal right now. We can open an issue for that with help wanted maybe.

tzarebczan (Migrated from github.com) reviewed 2019-02-12 00:52:58 +01:00

src/renderer/page/settings/view.jsx

tzarebczan (Migrated from github.com) commented

2019-02-12 00:52:58 +01:00

But it will either be null or a value - then we need to check for the case of a value. I can change this...

neb-b (Migrated from github.com) reviewed 2019-02-12 00:54:55 +01:00

src/renderer/page/settings/view.jsx

neb-b (Migrated from github.com) commented

2019-02-12 00:54:54 +01:00

Yeah.

this.setDaemonSetting('max_key_fee', isDisabled ? null : true);

I think that will work.

Yeah. ``` this.setDaemonSetting('max_key_fee', isDisabled ? null : true); ``` I think that will work.

tzarebczan (Migrated from github.com) reviewed 2019-02-12 01:32:30 +01:00

src/renderer/page/settings/view.jsx

tzarebczan (Migrated from github.com) commented

2019-02-12 01:32:30 +01:00

Just removed the top part...otherwise, true is not a valid value. When you choose the other option, it sets a LBC/USD amount. Changes have been pushed.

neb-b (Migrated from github.com) reviewed 2019-02-12 03:21:15 +01:00

src/renderer/page/settings/view.jsx

					
				@ -80,7 +82,7 @@ class SettingsPage extends React.PureComponent<Props, State> {

				  }

neb-b (Migrated from github.com) commented

2019-02-12 03:21:15 +01:00

This shouldn't need a client setting either.

neb-b (Migrated from github.com) approved these changes 2019-02-12 17:02:55 +01:00

Sign in to join this conversation.

No reviewers

neb-b

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-desktop#2277

				`@ -80,7 +82,7 @@ class SettingsPage extends React.PureComponent<Props, State> {`
				`}`