After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 732527 - Fix dead conditional
Fix dead conditional
Status: RESOLVED FIXED
Product: gnome-software
Classification: Applications
Component: General
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME Software maintainer(s)
GNOME Software maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-06-30 22:42 UTC by Michael Catanzaro
Modified: 2014-08-08 15:16 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Fix dead conditional (1.22 KB, patch)
2014-06-30 22:42 UTC, Michael Catanzaro 		rejected Details | Review
Improve update page state handling (4.32 KB, patch)
2014-07-06 15:52 UTC, Kalev Lember 		none Details | Review
Improve update page state handling (4.88 KB, patch)
2014-07-06 16:12 UTC, Kalev Lember 		none Details | Review

Description Michael Catanzaro 2014-06-30 22:42:50 UTC 
See patch, I guess this was removed to ease debugging and forgotten?

Or was it removed because it's now necessary for that page to refresh properly? I don't see any calls to gs_shell_updates_invalidate() which seems off.
Comment 1 Michael Catanzaro 2014-06-30 22:42:52 UTC 
Created attachment 279640 [details] [review]
Fix dead conditional

cache_valid is never read, and so gs_shell_updates_invalidate does
nothing. Looks like this was introduced for debugging and forgotten?

The effect of this change is to go straight to the list of updates when
visiting the updates page (if the cache is valid), rather than flashing
Checking for updates for a fraction of a second.
Comment 2 Kalev Lember 2014-07-06 15:47:44 UTC 
Review of attachment 279640 [details] [review]:

I tested this today and simply reinstating that conditional seems to break our somewhat fragile state handling in same cases.

I'll post an alternative patch in a minute, can you give it a try please?
Comment 3 Kalev Lember 2014-07-06 15:52:27 UTC 
Created attachment 279994 [details] [review]
Improve update page state handling

... to avoid flashing the "Checking for updates" label when switching
between different pages.
Comment 4 Kalev Lember 2014-07-06 16:12:01 UTC 
Created attachment 279995 [details] [review]
Improve update page state handling

... to avoid flashing the "Checking for updates" label when switching
between different pages.
Comment 5 Michael Catanzaro 2014-07-06 20:06:05 UTC 
Seems good to me; well, there are still plenty of bugs with this page, but I don't notice any regressions from your patch.
Comment 6 Kalev Lember 2014-08-08 15:16:27 UTC 
I did a bit more testing and found a few issues with my first patch. I've fixed those up now and pushed this as https://git.gnome.org/browse/gnome-software/commit/?id=26d430cbf141c08c34055c39af1ec65f943dacba