Skip to content

Add sticky channel option#67

Open
soullivaneuh wants to merge 1 commit intomaknz:masterfrom
soullivaneuh:sticky-channel
Open

Add sticky channel option#67
soullivaneuh wants to merge 1 commit intomaknz:masterfrom
soullivaneuh:sticky-channel

Conversation

@soullivaneuh
Copy link
Copy Markdown

Closes #40

With this option, all message will be sent to the default channel only.

This is useful for dev environment when you want to concentrate all test message to a specific test channel.

This was dicussed for a long time on #40 but we finally get an agreement with @Gummibeer at #40 (comment) 😉

@maknz Could you please review the code and merge it if it looks OK? Thanks.

@soullivaneuh
Copy link
Copy Markdown
Author

Added a note on readme.

With this option, all message will be sent to the default channel only.

This is useful for dev environment when you want to concentrate all test message to a specific test channel.
@Fabiencdp
Copy link
Copy Markdown

  • 1 !!

Copy link
Copy Markdown

@Gummibeer Gummibeer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except the single boolean cast everything is ok.

* @return bool
*/
public function isStickyChannel()
{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will force the result to be an boolean. Otherwise it can also be any other type.
return (bool) $this->sticky_channel;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the phpdoc, sticky_channel must be a boolean and developpers should respect this.

Casting anything to a boolean is not the right way IMO.

When this project will use PHP >=7.0, then we will be able to use strict type instead.

@soullivaneuh
Copy link
Copy Markdown
Author

Any news about this MR? Thanks.

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.

4 participants