search

maknz / slack

A simple PHP package for sending messages to Slack, with a focus on ease of use and elegant syntax.
BSD 2-Clause "Simplified" License
1.17k stars 204 forks source link

Add icon_url to slack payloads #1

Closed willwashburn closed 10 years ago

willwashburn commented 10 years ago

This PR allows you to send an icon_url with the payload so you don't have to use the default image.

maknz commented 10 years ago

Thanks for this, I was planning on implementing attachments shortly as well.

I'm a little concerned the send method is getting quite bogged down with parameters (it was already somewhat a bad design). I was thinking of changing to maybe something like:

// Send using the defaults in the config
Slack::send('Some text');

// Send but change e.g. the icon
Slack::withIcon('http://some.icon')->send('Some text');

// Send with an attachment
Slack::withAttachment($bigAttachmentArray)->send('Some text');

// Send with many attachments (replacing any already loaded up)
Slack::setAttachments($arrayOfAttachments)->send('Some text');

// Send as a different display name
Slack::as('R2D2')->send('Some text');
// synonymous with
Slack::withUser('R2D2')->send('Some text');

So we do away with the idea of defaults (which would be handled by config) and have chainable methods for overriding them per call.

If you like that style and direction, I'll be happy to build that out tonight and also implement some unit testing, and tag a 0.2 release?

willwashburn commented 10 years ago

@maknz I agree, that sounds like a good plan. Looking forward to the tests as well!

How do you envision the attachments working in the future? Would it make sense to have some abstraction for those to avoid complicated arrays?

The only thing I can think of right now is having some sort of an attachment object, but I'm not sure how that would look with your proposed changes.

Let me know how I can help!

maknz commented 10 years ago

An attachment object would definitely be the best design, but I wonder if we can keep the nice syntactic sugar. Maybe a Slack::attach() method could either take an attachment object, or take an array of parameters which under the hood creates an attachment object with that data, using defaults for parameters the array didn't specify. Then maybe for power users, Slack::setAttachments() could require an array of attachment objects.

maknz commented 10 years ago

Making some good progress on this, but taking longer than expected. Should have a PR up by the end of the weekend for review.