LBRYCommunity/lbry-desktop

Fork 0

Check if daemon version is greater than or equal to recommended, and modal edit. #378

Closed

hackrush01 wants to merge 2 commits from daemon_version_greater into master

hackrush01 commented

2017-07-22 17:21:50 +02:00

(Migrated from github.com)

Some quickfixes which enhance the user/developer experience.

kauffj (Migrated from github.com) reviewed 2017-07-22 17:21:50 +02:00

kauffj commented

2017-07-23 01:14:33 +02:00

(Migrated from github.com)

This is a good change generally, but I'm not convinced we want to allow greater version numbers on the daemon. If the app is shipped linked to a specific daemon version, then higher daemon versions are not necessarily backwards compatible, and could have unforeseen consequences.

hackrush01 commented

2017-07-23 06:04:37 +02:00

(Migrated from github.com)

I did consider that scenario, but IMO that is going to happen rarely, so for only those releases we would make it an equality check(one line modification), like the way it is done in other softwares.
Does this sound good?

tzarebczan commented

2017-07-23 06:17:45 +02:00

(Migrated from github.com)

Or have it be a setting, default value to equals. Maybe "developer mode"?

On Jul 23, 2017 12:04 AM, "hackrush" notifications@github.com wrote:

I did consider that scenario, but IMO that is going to happen rarely, so
for only those releases we would make it an equality check(one line
modification), like the way it is done in other softwares.
Does this sound good?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/lbryio/lbry-app/pull/378#issuecomment-317227373, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AHvpkWIfkWnD-uIxPbrlHfcKGi5BDdCXks5sQsZWgaJpZM4OgMvv
.

Or have it be a setting, default value to equals. Maybe "developer mode"? On Jul 23, 2017 12:04 AM, "hackrush" <notifications@github.com> wrote: > I did consider that scenario, but IMO that is going to happen rarely, so > for only those releases we would make it an equality check(one line > modification), like the way it is done in other softwares. > Does this sound good? > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <https://github.com/lbryio/lbry-app/pull/378#issuecomment-317227373>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AHvpkWIfkWnD-uIxPbrlHfcKGi5BDdCXks5sQsZWgaJpZM4OgMvv> > . >

hackrush01 commented

2017-07-23 08:14:46 +02:00

(Migrated from github.com)

Yes, I always thought a developer mode was long due(that could be done using a --dev flag), but I am not sure how many changes would a step like this require. But surely that would streamline the process for devs as well as users. But in the interim some sort of workaround could be implemented.

Yes, I always thought a developer mode was long due(that could be done using a `--dev` flag), but I am not sure how many changes would a step like this require. But surely that would streamline the process for devs as well as users. But in the interim some sort of workaround could be implemented.

kauffj commented

2017-07-26 00:40:33 +02:00

(Migrated from github.com)

Can you help me see the use case for a developer testing a higher version number of the daemon, but not wanting to update the packaging settings? Changing the daemon version is easy and being strict ensures that the developer is always coding against the daemon they intend to be.

tzarebczan commented

2017-07-26 01:23:20 +02:00

(Migrated from github.com)

Also, from a support perspective, it would be nice to have a quick solution for users to test that a certain bugfix is working in a later daemon. Although this shouldn't happen frequently, it's convenient.

All of this assumes that the higher level daemon does not break anything app related.

Recently with the various daemon big fixes, I've been using updated daemons in my Windows app installs in order to test and verify. As a windows user, it's nice to be able to drag and drop the daemon files for quick testing. Otherwise I would need to build from source or have the devs issue an app update along with daemon. Also, from a support perspective, it would be nice to have a quick solution for users to test that a certain bugfix is working in a later daemon. Although this shouldn't happen frequently, it's convenient. All of this assumes that the higher level daemon does not break anything app related.

kauffj commented

2017-07-26 01:34:06 +02:00

(Migrated from github.com)

@tzarebczan part of my hesitancy here is that a higher daemon version is basically promising, or at least strongly hinting, that it will break something app related.

However, you raised some good points, and I could see how testing a higher daemon via the app could be a quick way to assess potential issues.

Will make a call here soon.

(Also, I did notice on second glance that the version check is a straight string check, this also probably needs to change.)

@tzarebczan part of my hesitancy here is that a higher daemon version is basically _promising_, or at least strongly hinting, that it will break something app related. However, you raised some good points, and I could see how testing a higher daemon via the app could be a quick way to assess potential issues. Will make a call here soon. (Also, I did notice on second glance that the version check is a straight string check, this also probably needs to change.)

hackrush01 commented

2017-07-31 16:43:33 +02:00

(Migrated from github.com)

I think this can be closed as allowing for a new daemon version could create problems even for devs, so a manual allowance should only be done (if required), as pointed out by @kauffj. The second commit was broken into a new PR here.

kauffj commented

2017-08-01 15:25:44 +02:00

(Migrated from github.com)

I'm closing this since the modal changes got merged in the other PR.

@tzarebczan raised a good point, so I'm fine with adding this. But the following would need to change:

It has to be a proper version check. String checking on version numbers can fail ("0.10" < "0.9").
It has to give some kind of indicator or acknowledgement that you are running a future daemon. It shouldn't be possible to do silently.

I'm closing this since the modal changes got merged in the other PR. @tzarebczan raised a good point, so I'm fine with adding this. But the following would need to change: 1) It has to be a proper version check. String checking on version numbers can fail ("0.10" < "0.9"). 2) It has to give some kind of indicator or acknowledgement that you are running a future daemon. It shouldn't be possible to do silently.