Review "OpenWeather" version 108 - GNOME Shell Extensions

Review of "OpenWeather" version 108

Details Page Preview

OpenWeather

by skrewball

Display weather information for any location on Earth in the GNOME Shell

Extension Homepage: https://gitlab.com/skrewball/openweather

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

All Versions

Version	Status
121	Active
120	Rejected
119	Active
118	Active
117	Active
116	Active
115	Active
114	Active
113	Active
112	Rejected
111	Active
110	Active
109	Active
108	Rejected
107	Active
106	Active
105	Active
104	Rejected
103	Rejected
102	Active
101	Rejected
100	Active
99	Active
98	Rejected
97	Active
96	Active
95	Active
94	Active
93	Rejected
92	Active
91	Active
90	Active
89	Active
88	Active
87	Active
86	Active
85	Active
84	Active
83	Active
82	Rejected
81	Active
80	Active
79	Active
78	Active
77	Active
76	Active
75	Active
74	Active
73	Active
72	Rejected
71	Active
70	Active
69	Rejected
68	Rejected
67	Active
66	Active
65	Active
64	Active
63	Active
62	Active
61	Active
60	Active
59	Active
58	Active
57	Active
56	Rejected
55	Active
54	Active
53	Active
52	Active
51	Active
50	Active
49	Rejected
48	Active
47	Active
46	Active
45	Rejected
44	Rejected
43	Rejected
42	Active
41	Active
40	Active
39	Active
38	Active
37	Active
36	Active
35	Active
34	Rejected
33	Active
32	Active
31	Active
30	Active
29	Active
28	Active
27	Active
26	Active
25	Active
24	Active
23	Active
22	Rejected
21	Rejected
20	Active
19	Rejected
18	Rejected
17	Active
16	Active
15	Active
14	Rejected
13	Active
12	Rejected
11	Rejected
10	Rejected
9	Active
8	Rejected
7	Rejected
6	Rejected
5	Rejected
4	Rejected
3	Rejected
2	Rejected
1	Rejected

Previous Reviews on this Version

JustPerfection posted a review May 2, 2022

Basically, when a window is getting closed, still, the timeout callback can get triggered. 99.99% of the extensions don't use any timeouts in the prefs (there isn't so many use case for that) but if you have any timeout, it needs to be cleaned out in close. I think it's good that you sent the next version without any timeouts in prefs. Thatnks!

skrewball posted a review May 2, 2022

Perfect, I'll move forward without those timeouts. Thanks! :)

skrewball posted a review May 1, 2022

This version removes the legacy Lang class and updates libsoup code to v3. Also removes all darksky code, lots of bug fixes including the icon issues some users were having on PopOS. The next version I am rebuilding prefs.js using libadwaita.

JustPerfection rejected May 2, 2022

Great! Please fix these and send it again: 1. No need for spawn in line 931 (extension.js) since it is only supported in 40 and higher. 2. Remove all of the timeouts when the prefs window is getting closed: ```js prefswidget.connect('close-request', () => { // ... }); ```

skrewball posted a review May 2, 2022

Removed the spawn on line 931, but just wondering about the timeouts when the prefs window is getting closed. Do you mean line 163 and 164 in prefs.js? If so I'm not sure what timeouts are linked to them? I added those 2 lines because when in prefs clicking edit location or add location opens sub windows, they were not closing when clicking the close button on the titlebar and that did it, those connects are not linked to the main window. If you're referring to the timeouts from line 318 I'm not exactly sure why Jens had those there, they appear to be linked to the spinners I'm not sure why, perhaps they were needed in older versions of gnome? Was planning to take care of those when rebuilding prefs.js for libadwaita but I'll investigate now.

skrewball posted a review May 2, 2022

It seems the timeouts on the spinners and slider was used to prevent the value from continuously being saved while the user is changing the value. Removing those does not seem to hurt anything.

skrewball posted a review May 2, 2022

So I just realized that you probably meant to use a close-request signal on the main prefs window to remove the timeouts when the window closes rather than completely remove them? Apologies, lack of sleep lately I should have caught that lol. I remember you asking about the timeouts on the last version. The main difference I've noticed without them is that there can be a little delay when adjusting the spinners and slider. I already uploaded a new version with the timeouts completely removed, would it be better to reject that version too and re-add the timeouts while connecting to a close-request to remove them? Or we could just go ahead with them completely removed.