Where do you remove the timeouts in extension.js (on destroy or disable)?

fubar.js:L50 ``` host.connectObject('destroy', () => this.dispel(), onus(host)); ```

That makes it really hard to review and tbh most lines in your extensions seem cryptic.

This line of your review seems unactionable for me. If I can do nothing for the approval, feel free to reject.

Please make it obvious so that can be understood by reviewers: https://gjs.guide/extensions/review-guidelines/review-guidelines.html#code-must-not-be-obfuscated and that Python script should be a module written in gjs: https://gjs.guide/extensions/review-guidelines/review-guidelines.html#scripts-and-binaries

> https://gjs.guide/extensions/review-guidelines/review-guidelines.html#code-must-not-be-obfuscated I didn't minify or obfuscate the code. I had thought that putting stuffs that needed to be destroyed together would save the reviewer's time. > https://gjs.guide/extensions/review-guidelines/review-guidelines.html#scripts-and-binaries I cannot do that unless "glib-opencv" and "glib-tesseract" or their gobject introspection alternatives exist.

> I didn't minify or obfuscate the code. I had thought that putting stuffs that needed to be destroyed together would save the reviewer's time. Function names like "onus", "scapegoat", "dispel", "summon" or "symbiont" are confusing and very unnecessary. These aren't common programming terms and make it very hard to follow the code, or to guess what the function is supposed to do. A line like `let value = isEnabled ? doSomething() : doNothing();` is easily understood because the variables and functions tell you want they are. A line like `host.connectObject('destroy', () => this.dispel(), onus(host));` is not because the terms don't describe the variables or the functions. > I cannot do that unless "glib-opencv" and "glib-tesseract" or their gobject introspection alternatives exist. If you must use Python the guidelines allow for that, but you have to understand reviewers are chosen and volunteer because they know how to review JavaScript. Like much of your JavaScript, the Python script is very difficult to review because you have about a hundred lines like this in it: `s = list(filter(lambda x: x[4] > 0.002 and x[4] < 0.95, [x + (x[2] * x[3] / ar,) for x in map(cv2.boundingRect, cs)]))` I'm sure this is obvious to you and other Python developers, but knowing your code is being reviewed you could make this a lot easier on us volunteers. So here is my attempt at an actionable review: * Please use common programming terms like "observer", "disconnect", and "delegate". Avoid use confusing or uncommon terms like "symbiont", "dispel" or "scapegoat". * Please emphasize clean, readable code over dense code. If it is unavoidable, add short comments to help reviewers quickly understand the logic * If you must use another scripting language like Python, please assume the reviewer is "new" to Python. Again, avoid dense code when possible and if necessary add short comments

> Function names like "onus", "scapegoat", "dispel", "summon" or "symbiont" are confusing and very unnecessary. These aren't common programming terms and make it very hard to follow the code, or to guess what the function is supposed to do. I didn't mean to make things difficult for reviewers by using uncommon names, but many classes inherit from GObject/ClutterActor/Gnome Shell's code, and there have been undetectable bugs caused by unintentionally overriding the parent class with a common name, so I've taken to using `_name` or `uncommon_name` in inherited classes to avoid potential conflicts (i.e `summon` & `dispel` are to distinguish from `show` & `hide` in Clutter.Actor), but in non-inherited classes like ` Extension` do use `_delegate` (which is commonly used in Gnome Shell's code, also that's why using `scapegoat`). I think `Symbiont` is a little better than `ObjectDestroyWithAnother` or `DestroyTogether`... I had thought the name and function consistency was okay, but it seems not. > If you must use Python the guidelines allow for that, but you have to understand reviewers are chosen and volunteer because they know how to review JavaScript. Like much of your JavaScript, the Python script is very difficult to review because you have about a hundred lines like this in it: `s = list(filter(lambda x: x[4] > 0.002 and x[4] < 0.95, [x + (x[2] * x[3] / ar,) for x in map(cv2.boundingRect, cs)]))` For that particular new line, it's obvious just filtering on the array compared to old code. If there was a problem here, I think it would be a bug rather than malware. Most of this commit is renaming variables, so it seems to make these few new changes difficult to read, sorry for that. I have commented after any code that referenced or that I personally found difficult to understand. But it seems far from enough. TBH, the code is just as dense as before, not denser. A bit of a surprise that its density has suddenly become an issue.

There is a `Scapegoat Tree`(https://en.wikipedia.org/wiki/Scapegoat_tree) in CS.

> A bit of a surprise that its density has suddenly become an issue. What has changed is the review guidelines, which were made much more strict in response to community feedback. The long story is that when I start reviewing extensions, there were over 300 extensions waiting for review, so only extensions that were non-functional or very dangerous were rejected. Since then, the demand for GNOME to guarantee stability from third-party extensions has become very high. In other words, this partially my fault, because there are many extensions like yours that were approved based on a different standard. To meet the demand for stability, its necessary for reviewers to actually understand how an extensions works, but this is obviously hard with different coding styles and unfamiliar technology. So what's happened is your diff for this submission revealed code which is more difficult to review than the style of code expected from other extensions. We discussed and decided to approve the extension, because you obviously maintain and fix bugs regularly. We'd still like to see some improvements, so that it moves closer to how other extensions are reviewed. JustPerfection will explain specifics so we can figure something out :)

We can approve this one for this version but the variable/function naming should be changed for all of your extensions. For example, line 32 util.js. You may remember what `fl` is doing as the maintainer but we are reviewing so many lines of code everyday. That's why I'm saying that can make the review process harder. and the review page doesn't have IDE like tools that we can do ctrl+click to go to the definition. btw, I don't see any good reason for making the variable names cryptic (something like line 72 fubar.js). Are you trying to save some spaces (file size or line margin)? btw, naming (part of readability) is a very important part of the code: [discussion in GNOME Shell repo for function naming](https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2012#note_1377343). As you can see, `fillPreferencesWindow` is a better choice over `fillPrefsWindow` just for the sake of readability. Please fix those for the next version.

If they deserve that, I'd rather they stay rejected. Thanks. :-)

Ok. Let us know if you've decided to change that.