Review of "Brightness control using ddcutil" version 45

Details Page Preview

Brightness control for all the monitors detected by ddcutil This tool uses ddcutil as backend for communication with your display. Read setup instructions from: https://github.com/daitj/gnome-display-brightness-ddcutil/blob/master/README.md

Extension Homepage
https://github.com/daitj/gnome-display-brightness-ddcutil

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. Please use `console.*` instead of `log()`: https://gjs.guide/extensions/upgrading/gnome-shell-45.html#logging 2. All of those timeout functions supported on 45 (line 41-79 convenienceExt.js). Please remove them. 3. Also null out `this.settings` in disable: https://gjs.guide/extensions/review-guidelines/review-guidelines.html#destroy-all-objects 4. `destroy()` not `destory()`: - line 135 extenision.js - line 272, 358 indicator.js 5. Shouldn't be `console.log()`, should be `console.debug()` (line 197-200 extension.js). 6. Timeout should be removed on disable (line 148 and 544 extension.js): https://gjs.guide/extensions/review-guidelines/review-guidelines.html#remove-main-loop-sources
themightydeity posted a review
I do not have GNOME 45, running I relying on a pull request by contributors and their feedbacks. I have fixed most of these issues, once I get feedbacks from the contributors I will upload a new version. 5. By default extension won't output anything to log, you need to enable it peferences for `brightnessLog` to work. How does console.debug work in normal session, what are the steps to enable debug mode, I guess if I replace console.log with console.debug, I need to add that step also in my issue template. 6. for line 148, the clearInterval in line 128 should remove the timeout, do I need to do something else also?
JustPerfection posted a review
5. `console.debug()` only show up in the logs if the debug mode is enabled (`G_MESSAGES_DEBUG=all` or enabled GLib log debug). So, You can use `console.log()` if users have controls over the logs in your extension's settings. 6. Missed that. It's ok then.