-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modularity of behaviors? #39
Comments
Ah, i think i see the distinction -- tell me if this is right:
And services are needed because multiple interfaces may use similar patterns of action, so |
But for this idea of encapsulating the behaviors each in one file, talk with @ccpandhare -- see how this is being done in the modules over there: https://github.com/publiclab/image-sequencer/tree/master/src/modules |
@jywarren you got it. Services are basically groups of behaviors grouped on the basis of function. eg: help behavior comes under the chatbot service and so on. I am willing to discuss this. Hopefully, we'll make the codebase more legible for new contributors. I agree that the |
That's great -- can we build out a part of the README that says this very specifically? I had been confused about services vs. interfaces or behaviors. Maybe one section near the top that says So this issue could be broken into:
|
OK, so copying in my comment from #13, this is fleshing out the 2nd checkbox from my last comment: Let's get the behaviors into their own folder and standardize the way to add a new behavior accordingly, i.e. (and adding this to the README):
This should make it really easy to add new behaviors, and to bulk that out before we add a new interface. Make sense? |
And now that I look at it, |
And then https://github.com/publiclab/plotsbot/blob/master/services/chatbot.js#L38 |
Ah, oops, it is the real person's nick. Better change that to clarify! Maybe... |
@jywarren great ideas. Would love to discuss this with you in person if possible and get this sorted. Then I will change the |
Hi, Ujjwal - I'm sorry but I don't have time today to talk in person, and
in general I often don't, as you have probably realized. For this I
apologise but there are a lot of projects going on right now.
Do you think you could meet with Ananyo and discuss things, and post
questions and clarifications on GitHub so I can provide feedback when I'm
able? You two are a great team and your work to date is great. I'm sure
you'll be able to figure things out.
…On Jun 17, 2017 10:22 AM, "Ujjwal Sharma" ***@***.***> wrote:
@jywarren <https://github.com/jywarren> great ideas. Would love to
discuss this with you in person if possible and get this sorted. Then I
will change the help and greet behaviors to conform to our new standards
and add this specification to the README. This would definitely improve the
workflow, make contributor's jobs and our lives easier.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJwil-eKwr3ajweK8wExBMd00uL94ks5sE-EYgaJpZM4N84l5>
.
|
@jywarren okay, I totally get it. Will post the results of our discussions on Github today. I have a few ideas. Hopefully, you will like it. |
Hi @ryzokuken @jywarren I guess a proper tree structure regarding the modules should be thought of so that we are clear with the approach. |
@ananyo2012 that's a great idea. We could make a simple diagram to explain it. |
+1!
…On Sat, Jun 17, 2017 at 2:39 PM, Ujjwal Sharma ***@***.***> wrote:
@ananyo2012 <https://github.com/ananyo2012> that's a great idea. We could
make a simple diagram to explain it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ3bjPZlfNUR63BjNfD9pohnZzPiqks5sFB1YgaJpZM4N84l5>
.
|
Our current directory structure
|
A possibility
|
A rough specification for behaviors
We could add the |
What about "trigger" instead of type and " keyword" instead of identifier?
I don't feel strongly about it but just a suggestion.
…On Jun 18, 2017 4:29 AM, "Ujjwal Sharma" ***@***.***> wrote:
*A rough specification for behaviors*
type: String => what kind of behavior is it. This will allow the bot to determine when to trigger that particular behavior. For our case, we have two types of behaviors right now. `join` behaviors eg: greet and `message` behaviors eg: help.
identifier: String => only applies for behaviors with type `message`. This identifier tells the bot when to trigger that particular behavior (when the identifier is a part of the message). For example, the identifier for the `help` behavior is the keyword "help".
action: Function => This is the function that will be called by the bot with appropraite standard arguments when the behavior is triggered. These "standard" arguments will be specified shortly.
We could add the class Behavior to a folder called models or inside the
behaviors folder itself. We could either write a class MessageBehavior
extends Behavior to include the extra property to just include the
indetifier property in the original class and set it to undefined
whenever unneeded or set it anyway and not use it. (I prefer the last one)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ05ulbc5Fq1f-yidyONqifrW2uH0ks5sFN_ygaJpZM4N84l5>
.
|
@jywarren any word that captures the meaning of the variable best would work for me. What are your thoughts about the plan overall? |
I like it -- did you get rid of /services/ for some reason? I thought that
was fine as it was. I also think to avoid too many folders, we could have
/behaviors.js and /behaviors/____.js
…On Sun, Jun 18, 2017 at 8:37 AM, Ujjwal Sharma ***@***.***> wrote:
@jywarren <https://github.com/jywarren> any word that captures the
meaning of the variable best would work for me. What are your thoughts
about the plan overall?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ8t0JqVyd5FiYwdDqcV3N55G0qoWks5sFRofgaJpZM4N84l5>
.
|
@jywarren As services were nothing but groups of behaviors, I don't think we'd need services anymore. What about the specification though, does that sound okay to you? |
It sounds great. Let's start a PR, with the documentation, and build it out!
…On Sun, Jun 18, 2017 at 12:28 PM, Ujjwal Sharma ***@***.***> wrote:
@jywarren <https://github.com/jywarren> behaviors.js and behaviors/*.js
sounds great too. Let's finalize it then.
As services were nothing but groups of behaviors, I don't think we'd need
services anymore.
What about the specification though, does that sound okay to you?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ63B961AUs92VquaMxc4EsbK9P5kks5sFVAvgaJpZM4N84l5>
.
|
The standard arguments to behavior actionsJoin behaviors
Message Behavior
Note: Notice how the bot's nickname is referred to as |
I think if it regularly shows up as bot.nick, that's fine, but this.nick is
not great. Can you internally alias "this" to "bot" wherever you can? It's
good practice anyway not to blindly use "this" since it has different
meanings throughout a source code file.
…On Jun 18, 2017 12:42 PM, "Ujjwal Sharma" ***@***.***> wrote:
The standard arguments to behavior actions Join behaviors
1.
channel: String => The channel on which the user joined. As DM
channels cannot be joined, this would be more useful once the bot is
deployed on multiple channels (which it will be soon when it hits
v1.0.0)
2.
username: String => The username of the user who just joined.
Message Behavior
1.
from: String => the nick of the user who sent the message.
2.
to: String => the channel in which the message was recieved. If the
message was a DM, this would be equal to the bot's nick
3.
message: Array => The different words of the message, stripped of all
punctuation. Also, the nick of the bot and the identifier of the behavior
are removed from the list of the words.
*Note:* Notice how the bot's nickname is referred to as nick while other
users' nicknames are referred to as username. @jywarren
<https://github.com/jywarren> are you satisfied with this terminological
distinction?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ0-F4KRMRLGrGK0JjFpPWHrkwoh7ks5sFVNZgaJpZM4N84l5>
.
|
@jywarren Sure, why not. |
@jywarren while I am starting to work on the PR, should we make this a |
yep, agreed, api-breaking. Super!
…On Sun, Jun 18, 2017 at 1:07 PM, Ujjwal Sharma ***@***.***> wrote:
@jywarren <https://github.com/jywarren> while I am starting to work on
the PR, should we make this a major version change? I mean, there are
breaking changes in the application API, right? I am new to semver.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ3qJ0JGlQ44_US0Hfdnk98ul_Kz7ks5sFVlSgaJpZM4N84l5>
.
|
Hi, @ryzokuken - so are you calling things like
greet
andhelp
"services" then, and not "behaviors"?In any case, I see the
help
behavior is insideutil.js
and I wonder if we could make each Behavior its own submodule, included via arequire()
and with its own internal/external API. There could be a folder at/src/behavoirs/____
which includes each one. Then we could just have a listing of them required in, here:https://github.com/publiclab/plotsbot/blob/master/utils.js#L32
The text was updated successfully, but these errors were encountered: