Review of "Persian Calendar" version 113

Details Page Preview

Shows Persian date in the top panel. It shows: 1. Persian calendar 2. It can show, today is a holiday or not! 3. Show notification onDayChanged! 4. Date converter between Persian, Gregorian and Lunar Hijri 5. Events: 5.1. Official solar events. 5.2. Official lunar events. 5.3. Official international events. 5.4. Traditional Persian events. 5.5. Persian personages. Please “rate” here and “star” the project on GitHub. Please open an issue on GitHub if you found something or have an idea!

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.

All Versions

Previous Reviews on this Version

JustPerfection rejected
EGO extensions cannot use `gdm` session mode: [EGO Review Guidelines: Metadata Session Modes](https://gjs.guide/extensions/review-guidelines/review-guidelines.html#metadata-json-must-be-well-formed) Also, please remove `session-modes` or follow the `session-modes` rules: [EGO Review Guidelines: Metadata Session Modes](https://gjs.guide/extensions/review-guidelines/review-guidelines.html#session-modes)
omid posted a review
There are 3 points in the guideline. 1. It MUST be necessary for the extension to operate correctly. 2. All signals related to keyboard events MUST be disconnected in unlock-dialog session mode. 3. The disable() function MUST have a comment explaining why you are using unlock-dialog. So, I think #1 is fine. #2 is also fine, because there is no keyboard event in the extension. So for the last one, I just need to have a comment in the `disable` function? Actually not like what the guideline said, "In rare cases, it is necessary for an extension to continue running while the screen is locked." It's not "necessary" for this extension. But it's a nice to have.
JustPerfection posted a review
Yes, the comment should be added to the disable function. And the signal in line 127 `extension.js` needs to be disconnected on disable. btw, I don't recommend having session modes for this extension since extensions with `unlock-dialog` can get disabled and re-enabled multiple times during unlock session mode. That can be a little too much just for "nice to have". but that's your choice. I understand some users like to have the date in unlock dialog.
omid posted a review
It does disconnect. Everything inside `this.event_hooks` array will be disconnected on disable. Ok, I'll remove unlock-dialog then 🙏
JustPerfection posted a review
Thanks! btw, in line 533, `ext._settings` is getting disconnected not `Main.sessionMode`.
omid posted a review
Oops, yes, you are right... I'll rename the variable name for future to remember that. Thank youuuu <3