Review of "Screenshot Tool" version 71

Details Page Preview

Conveniently create, copy, store and upload screenshots. Please log out and log in again after updating.

Extension Homepage
https://github.com/OttoAllmendinger/gnome-shell-screenshot/

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
1. Don't use `var`. Use `let` and `const` instead. Use `export class` when you want to export class. 2. Too much for the constructor which breaks multiple rules (line 1425-1428 `extension.js`): - [EGO Review Guidelines: Initialization](https://gjs.guide/extensions/review-guidelines/review-guidelines.html#only-use-initialization-for-static-resources) - [EGO Review Guidelines: Destroy](https://gjs.guide/extensions/review-guidelines/review-guidelines.html#destroy-all-objects) 3. Why not just doing this in `enable` instead of line 219-229 `extension.js`: global scope: ```js let _ = null; ``` enable: ```js _ = this.gettext; ``` disable: ```js _ = null; ``` You can also do that for `prefs.js`
oal posted a review
(1) these anachronism come from certain dependencies I added. If you insist I can vendor them and use modern syntax. (2) and (3) will fix
oal posted a review
(3): this turns out to be harder than I thought. The original source for this extension is written in typescript and bundled with rollup. I'm uploading the compiled artifact here. While I can `export let _ =` from a module, I cannot reassign from an import site. If you don't mind I would like to keep it as is.