Skip to content
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

/command support #17

Closed
JasonEtco opened this issue Jan 13, 2019 · 5 comments
Closed

/command support #17

JasonEtco opened this issue Jan 13, 2019 · 5 comments
Labels
new feature A new feature being added or proposed

Comments

@JasonEtco
Copy link
Owner

JasonEtco commented Jan 13, 2019

It'd be great to provide a /command abstraction - something like:

// In an issue/PR comment, `/pizza dominos pepperoni`
toolkit.command('pizza', async args => {
  // args -> ['dominos', 'pepperoni']
})

Or maybe...

// on instead of command
toolkit.on('pizza', async args => console.log(args))

Memorializing a conversation in this tweet:

image


I think that @mscoutermarsh should work on this 😎😈

@JasonEtco JasonEtco added the enhancement A change to an existing feature label Jan 13, 2019
@JasonEtco
Copy link
Owner Author

Hi @mscoutermarsh cc this tweet

@JasonEtco
Copy link
Owner Author

JasonEtco commented Jan 28, 2019

Random thought - this'd be a weird API, because we'd need to ensure that its only triggered by issue_comment events, and that thats clear in the docs. Internally, that might be:

class Toolkit {
  public command (name: string, handler: () => void) {
    if (this.context.event !== 'issue_comment') return
    // ...
  }
}

Folks can use the events options in the Toolkit constructor, but that's probably not enough.

@JasonEtco JasonEtco added new feature A new feature being added or proposed and removed enhancement A change to an existing feature labels Feb 2, 2019
@macklinu
Copy link
Contributor

// In an issue/PR comment, `/pizza dominos pepperoni`
toolkit.command('pizza', async args => {
  // args -> ['dominos', 'pepperoni']
})

+1 to this API. I feel like registering a command like this would be more explicit than a generic event emitter/subscription API (e.g. toolkit.on('pizza')).

To clarify #17 (comment), are you stating that the only event a command can respond to is the issue_comment event and that the docs should explicitly state that? I think that makes sense as a potential user of this feature - can't think of another use case right now where a slash command handler would respond to a different GitHub event type.

@JasonEtco
Copy link
Owner Author

@macklinu thanks for sharing your thoughts!

To clarify #17 (comment), are you stating that the only event a command can respond to is the issue_comment event and that the docs should explicitly state that?

Yeah, it'd be to avoid a "gotcha" where folks use the API in an action that cannot, by definition, trigger the command. I'd want to avoid people accidentally thinking that a push might trigger a /command 😁

It might be worth building up a list of what those events are - issues, issue_comment, pull_request, commit_comment, pull_request_review_comment, pull_request_review I think? Basically anything that can have a "body" 🤔

@tcbyrd
Copy link

tcbyrd commented Feb 26, 2019

@JasonEtco We should also ensure it doesn't respond to bots, to prevent infinite loop situations (helpful when a Bot responds with a list of commands you can use)

We do this in the jira middleware:

if (context.payload.sender.type === 'Bot') {
  return
}

@JasonEtco JasonEtco mentioned this issue Feb 26, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature A new feature being added or proposed
Projects
None yet
Development

No branches or pull requests

3 participants