Skip to content

Disclose publisher option#73

Open
al7373 wants to merge 2 commits intovoryx:masterfrom
al7373:disclosePublisher_option
Open

Disclose publisher option#73
al7373 wants to merge 2 commits intovoryx:masterfrom
al7373:disclosePublisher_option

Conversation

@al7373
Copy link

@al7373 al7373 commented Jan 14, 2018

@davidwdan
Here is a proposed fix for Issue #25.

Please let me know if you have any recommended changes.

Thanks,
Alain

@ThePixelDeveloper
Copy link

This goes well with #79. Hope to see this included as well. 👍

@ThePixelDeveloper
Copy link

@al7373 Mind resolving the conflicts?
@davidwdan If the conflicts are resolved would this be good to merge?

Copy link
Member

@mbonneau mbonneau left a comment

Choose a reason for hiding this comment

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

This looks good except that it should only set the option if set in the annotation. This also needs to be reworked into the current master branch.


//Subscribe Options
$subscribeOptions = [
'disclose_publisher' => $disclosePublisher
Copy link
Member

Choose a reason for hiding this comment

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

I would like to see disclose_publisher only set if there is an option set, not for every subscribe.

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