NewXKitBot 2.0 #4
Conversation
| ### What is New-XKit Bot? | ||
| New-XKit Bot is the team behind [New XKit](https://github.com/new-xkit/XKit)'s helping little assistant, written in JavaScript for [Node.js](https://nodejs.org/). It's here to make the lives of the New-XKit team just a tiny bit easier! |
There was a problem hiding this comment.
This could be misread as "(New-XKit Bot is the team behind New XKit)'s helping little assistant" rather than "New-XKit Bot is (the team behind New XKit)'s helping little assistant". For clarity, I would rewrite this as
New-XKit Bot is New XKit team's little assistant
Wolvan
force-pushed
the
rewrite_1
branch
2 times, most recently
from
3a06d01 to
3598bab
Compare
|
Made your changes top the README.md, @bvtsang |
| 1. Install [Node.js](https://nodejs.org/) | ||
| 2. Clone this repository into a folder | ||
| 3. Copy the ``default.js`` file in the ``config`` subfolder and rename it to ``local.js`` |
There was a problem hiding this comment.
GitHub's Markdown Basics document recommends using single backticks (`).
|
The lack of in-line documentation makes things a little tricky to understand what's going on, particularly in the cases where RegEx is being used to transform the messages. I would also still like to see thorough documentation as to how the NewXKit bot is set up and run on our infrastructure. It doesn't necessarily need to be a part of this project, but formal deployment instructions would be helpful. Generally, having monolithic do-everything files make it more difficult to reason about a project. Splitting the code up by concerns and making things more modular would go a long way to help readability and understanding. These have just been my initial thoughts and comments, and do not constitute a formal code review. |
| @@ -1,3 +1,24 @@ | |||
| # NewXKitBot | |||
| # New-XKit Bot | |||
There was a problem hiding this comment.
I think we standardized on New XKit in prose and new-xkit in code, fyi.
|
would be nice to have this linted |
Since this is a rewrite from scratch, clear out everything and restore a clean workspace
After clearing out the old code, re-init the repository with a proper README.md, a LICENSE.md (GPL-2.0) and a .gitignore for NodeJS projects
| } else { | ||
| command.not_allowed(user, gitter, irc); | ||
| } | ||
| }.bind(this, from, args, command_data)); |
There was a problem hiding this comment.
Actually, that bind is necessary iirc. Since I am doing an async call here and I have 2 async calls collide it can lead to issues. I had that before
There was a problem hiding this comment.
as long as you don't use this in your anonymous functions, you don't need to use bind. Also, remove the (user, args, command) parameters from the argument list, they're in the closure context of the function
There was a problem hiding this comment.
I have from, and args and command_data I use though. The loop progresses, overwriting those variables, so when a callback gets called it messes up. See Screenshot
This happened to me before doing the bind
There was a problem hiding this comment.
right, but from, args and command_data don't change—they're static to the anonymous function context that's generated on line 96
These 2 clients are wrappers for the bigger node-irc and node-gitter modules that do most of the heavy lifting. Since only a few events and functions are needed from each, these 2 wrapper modules gives convenient access to them and make opening a connection easy.
This module combines the GitterChat and the IRCChat module to create a functional bridge between these 2 services. Each GitterIRCBridge represents a single pair of IRC and Gitter Channel.
Should be self-explanatory, write a default configuration file which can be extended through a local.js.
This is the bot's core to bootstrap it. Config loading and, for now, GitterIRCBridge-initializing are done from this core file
Run "npm init" to create the package.json for easy bot installation and meta-data
Put the license information in each source file the way GPL-2.0 says and remove the "How to apply this license" part from LICENSE.md
I decided to do a full rewrite of the bot, in turn cleaning it up and making it easier to use and to configure