noticed icon indicating copy to clipboard operation
noticed copied to clipboard

Slack delivery method inherits Webhook

Open hmdros opened this issue 2 years ago • 1 comments

Hi Chris,

Me and @toppa are making contributions on hacktoberfest.

Our PR is based on @vickyonit 's previous suggestion on the issue: https://github.com/excid3/noticed/issues/213

We had the Slack delivery method inherit the Webhook class so we could reuse some methods.

hmdros avatar Oct 18 '22 20:10 hmdros

Hi @excid3 - we'd be happy to do more on this. We started by trying to refactor the Webhook class from @vickyonit 's PR, to match the convention of the project (i.e. we started with a webhook subdir containing a base.rb class, etc). But this approach interfered with the abstraction logic for determining the delivery method, which won't work with that approach (at least not without some redesign).

Also you could treat the Webhook class more like an interface, by having all the delivery methods inherit from it, but that might be getting too Java-y. Anyway, let us know what you think and we'd be happy to proceed based on your feedback.

toppa avatar Oct 18 '22 20:10 toppa