Review of "Persian Calendar" version 93

Details Page Preview

🗓️ Displays Persian (Iranian/Jalali) calendar in the top panel It offers: 1. Displays the Persian/Iranian/Jalali calendar 2. Holiday indicator 3. Day change notifications 4. Converts dates between the Persian, Gregorian, and Hijri (lunar) calendars 5. Event listings: 5.1. Official solar events 5.2. Official lunar events 5.3. Official international events 5.4. Traditional Persian events 5.5. Notable Persian figures This extension writes to clipboard by user interaction. Please “rate” 💓 the project here and give it a 🌟 on GitHub. If you encounter any issues or have suggestions, feel free to open an issue there on GitHub! Keywords: فارسی / جلالی / تقویم / ایران / ایرانی / گاهشمار / گاه‌شمار / گاهشماری / گاه‌شماری / شمسی / خورشیدی / قمری / میلادی / هجری / Iran / Iranian / Jalali / Persian / Calendar

Extension Homepage
https://github.com/omid/Persian-Calendar-for-Gnome-Shell

No comments.

Diff Against

Files

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

Shexli (experimental)

Shexli has not produced a result yet.

All Versions

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

Previous Reviews on this Version

omid posted a review
Hello :) If you remember, it's related to having multi-language separated from Gnome-shell and OS language. The file related to this is the `utils/gettext.js`. I also have a simple caching, but not sure if a `let` global variable is a good practice or not. (I checked and the cache system works generally)
omid posted a review
(I just saw "locale/fa/LC_MESSAGES/persian-calendar.po~" file, it's not a big deal, I will remove it for the next releases)
JustPerfection rejected
Hi, 1. You cannot create objects in global scope which is the same as init (line 4 utils/gettext.js): https://gjs.guide/extensions/review-guidelines/review-guidelines.html#only-use-init-for-initialization 2. I missed this one before. You cannot create gsettings objects in global scope (line 10 extension.js). Move that line to enable and null that out in disable. 3. Also move line 5 (pref.js) to `App.constructor`. 4. Remove `locale/fa/LC_MESSAGES/persian-calendar.po~`. 5. Please inactivate version 92 after next version approval.
omid posted a review
Thanks. 1. So what is the idiomatic way of doing this? Create a variable in init and pass it around? In this case, what about perf.js file? It's ugly. Maybe I can ignore caching the gettext result! I'll do all others, thanks :)
JustPerfection posted a review
You have many options. Dependency injection is one of the way. Or you can create a settings module like this: ```js let settings; function getSettings () { if (!settings) { settings = ExtensionUtils.getSettings(); } return settings; } function destroy() { settings = null; } ``` on `disable()` you should use the `settings.destroy()`.
omid posted a review
Oh, thanks. So it's legal then. So I'll do this for the gettext cache. I think there is no need to destroy it for prefs, am I right?
JustPerfection posted a review
For prefs, just do this in the `App.constructor()`: ```js this._settings = ExtensionUtils.getSettings(); ``` The settings module I was suggested is more suited for `settings.js`. If you want to use it in prefs, you MUST destroy it in window close: ```js window.connect('close-request', () => { // destroy here }); ``` If you don't know how to get the window object: [42](https://gjs.guide/extensions/upgrading/gnome-shell-42.html#resize-preferences-window) [40 and 41](https://gjs.guide/extensions/upgrading/gnome-shell-40.html#get-prefs-window-and-resize-it)
omid posted a review
Could you please check the changes here, if this way is valid, then I'll upload a new version: https://github.com/omid/Persian-Calendar-for-Gnome-Shell/compare/master...fix_ego_comments I'm not sure about prefs.js, I think it'll free the memory when user closes it.
JustPerfection posted a review
AFAICS, it's ok. Send it here and I'll check that again. and yes, `App` object should be garbage collected when the prefs window is getting closed.