[02:21:53] hi [02:22:12] Please, someone could do a code review of [02:22:19] https://commons.wikimedia.org/wiki/User:Wilfredor/QICvote.js [02:22:20] thanks [02:23:09] it works fine, however, I would like to improve the readability [02:44:32] i can use ES6? [02:47:43] if you wish to improve readability, I suggest checking out https://www.mediawiki.org/wiki/Manual:Coding_conventions (and the js-specific conventions) used for the MediaWiki codebase itself. Also, add comments and perhaps re-order things for logical flow from top-to-bottom. [03:07:26] Yes, I will add the code convention and the order. I was talking about some semantic improve [03:20:23] Skizzerz could you take a look? [03:21:14] forget the comments, and look the code itself, I will add the comments after [03:22:42] one thing that stands out to me as I scroll down is the inconsistent indenting [03:22:46] " padding-top: 15px;" + [03:22:46] " padding-left: 15px;" + [03:22:46] " border: 1px solid #c8ccd1; " + [03:23:23] the indenting in numerous places makes it hard to read [03:23:36] hard/harder [03:24:54] well, usually I copy and paste it in a JS editor because the wiki page is not good to show code [03:25:34] maybe not, but it's better than it was years ago ;) [03:25:46] and it will show things properly aligned if consistent types and amount of leading whitespace is used [03:28:16] Reedy, done [03:28:50] definitely helps :) [03:29:34] :S [03:32:07] yep, inconsistent indentation, lack of braces, etc. is why I initially linked the coding conventions. If you want to make it easier for others to follow the code, those sorts of things help a lot. The other thing I'll mention is that you may find it beneficial to move the CSS parts into a .css subpage and load it there rather than embedding it as a string; that will net you CSS syntax highlighting and other features that make it easier to [03:32:07] examine [03:32:58] and comments help illustrate *why* you're doing something some certain way, which also helps others being able to read your code [03:34:08] Skizzerz, I would like to know if you saw the latest version [03:34:10] (now if you were not talking about code readability but readability of the final product, that was not very clear from your initial question asking for a code review. If you want the final product to be reviewed, screenshots are helpful) [03:34:44] My question is more if there is any semantic problem beyond the beauty of indentation [03:35:41] https://commons.wikimedia.org/wiki/Help:Gadget-QICvote [03:37:03] You will find screenshots on the help page [03:40:09] Yes I will create a css subpage [03:40:58] the reason I haven't done it is that it is more difficult to ask to change the script when there are multiple dependencies [03:41:06] well "It prevent edit conflicts" in the top block is incorrect [03:41:20] although it does likely reduce the likelihood of them happening, I don't see anything that outright prevents them [03:41:35] decreases? [03:42:13] (completely preventing edit conflicts is probably impossible, as an fyi) [03:42:26] no one has had an issue conflict, it is practically impossible (this script has been in use for more than a year) [03:42:51] sure, I don't disbelieve that. Just saying that edit conflicts are still possible :) [03:43:04] yes sure, thanks [03:43:55] there is a low probability but nothing is completely corrected, you are right [03:44:01] beyond that, nothing really stands out at me. Code quality wise there's a *lot* of typos [03:44:59] you also have a handful of variables you define but only use in one place [03:45:08] do you speak of misspelled words? [03:45:11] (and the one place they're used in is quite far from the place they're defined) [03:45:14] yes [03:46:40] weird mixing of inline styles in HTML blocks with CSS classes, best practice would be to make them all classes [03:47:12] not entirely sure how nitpicky you want the review to be [03:47:31] Yes, I was also thinking of creating another javascript file to define the html blocks, another for the labels [03:48:34] The review can be as tiring as possible [03:49:50] I do not know if it is possible that you see something that goes beyond the form [03:50:12] I'm talking about logic, not spaces, files, and hardcoding [03:51:23] when you ask someone to look at "readability" you are talking about things like spaces, files, and hardcoding [03:51:43] all of that code style stuff contributes to how "readable" the code is, and is very important if you want other people to understand and maintain the code after you move on [03:51:49] I think it was a problem of translation of this concept [03:52:33] for testing logic, having someone look at the code in isolation will catch some issues but most will go uncaught. You need to actually run the code and go through test cases to ensure everything is working as expected [03:54:31] Thank you very much good night [16:47:46] Hi, the job 2141375 on the Toolforge grid seems stuck, I can't stop/delete it. [16:48:20] "qstat -j 2141375 -ext -xml" gives me: "queue instance "continuous@tools-sgeexec-0939.tools.eqiad.wmflabs" dropped because it is temporarily not available" [16:48:50] Reedy, bd808: ^ [16:49:02] Ah shoot the correct channel is #wikimedia-cloud, sorry. [16:49:14] Normally #wikimedia-cloud would be best for Toolforge etc but I'm sure they'll come soon [16:56:00] thanks [17:06:53] thib: I force killed it for you. [17:08:09] bd808: thanks!