Skip to content

Conversation

@sjparkinson
Copy link
Contributor

Initial draft for resolving #14.

Depends on / based off #15.

Adds the purge method to ProductInterface.

@sjparkinson sjparkinson modified the milestone: v0.1.2 Aug 18, 2015
@sjparkinson sjparkinson force-pushed the feature/purge-queue branch 3 times, most recently from d8228ac to dc5bef3 Compare August 18, 2015 17:22
@sjparkinson sjparkinson force-pushed the master branch 2 times, most recently from d626d09 to 97cab30 Compare August 18, 2015 17:29

Choose a reason for hiding this comment

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

Why the less specific PHPDoc argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personal preference as the type hint is for array, but happy to undo.

Choose a reason for hiding this comment

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

ah, I see. I'm sure it's fine then.

@mathieupinet
Copy link

looks good otherwise (not tested).

@sjparkinson sjparkinson force-pushed the feature/purge-queue branch 2 times, most recently from e936770 to 86956d0 Compare August 20, 2015 15:39
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given the talk today, any thoughts on this @wpillar?

Copy link
Contributor

Choose a reason for hiding this comment

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

Here are my thoughts:

  • Client implements ProducerInterface and ClientInterface, so adding purge() to ProducerInterface does not unreasonably affect that class.
  • You've already got segregated interfaces in ProducerInterface and ClientInterface and since Client implements both, the only consideration really is how people might extend this code. Are we burdening people who might write separate ProducerInterface implementations with a method they can't implement? My thinking here is if we could reasonably assume that all queue implementations have a way of purging then it's fine to put this on ProducerInterface, as a purge operation is similar to a send operation. If we think this assumption is unreasonable then you can segregate further with a PurgeableInterface.

What's your thinking on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thinking here is if we could reasonably assume that all queue implementations have a way of purging then it's fine to put this on ProducerInterface, as a purge operation is similar to a send operation.

So far it's 👌, but I doubt every queue implementation supports purging.

We're not 1.0.0, so I'm going to go with this and see what Mat thinks if he makes use of it.

sjparkinson added a commit that referenced this pull request Aug 28, 2015
@sjparkinson sjparkinson merged commit 4226d07 into master Aug 28, 2015
@sjparkinson sjparkinson deleted the feature/purge-queue branch August 28, 2015 13:26
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