Skip to content

Conversation

@jakewilkins
Copy link
Collaborator

👋 howdy!

So I didn't totally mean to push this branch to this repo, I didn't expect that to work. If you'd like me to close this and open this off of a fork let me know and I will!

Also true to the spirit of a good open source PR I haven't written any tests 😀

If this is something you're interested in going anywhere with let me know and I'll write some tests.

If this isn't what you had in mind for this no worries, if there's something I can change to make it so let me know or this can just live in my memory as a fun afternoon hacking.

Example:

require "underway"
require "sinatra/base"
require "underway/sinatra/event_server"

Underway::Settings.configure do |config|
  config.app_root = __FILE__
  config.config_filename = "config.json"
end

class App < Sinatra::Base
  include Sinatra::Underway::EventServer

  on("issues.*") do |event|
    %i|repository issue sender installation|.each do |attr|
      p event.send(attr)
    end
  end
 
  on("pull_request.opened") do |event|
    # do something else.
  end
end

To do:

  • I wrote this. Do you want this?
  • If yes to question 1, write tests, any changes you'd like.

@jamesmartin
Copy link
Owner

  • I wrote this. Do you want this?

@jakewilkins 💯

Thanks for opening this PR. I love the direction you've chosen for the API here. I wonder if there are any other event-driven Sinatra gems that use the same interface (E.g. on) that we should avoid? Maybe just adding a namespace would be sufficient. What do you think?

Also, thanks for offering to add tests. As you can see, the test coverage is not exactly stellar in this project right now, but I'm happy to help if you want to pair on it.

Base automatically changed from master to main September 1, 2020 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants