Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

ServiceBusPlugin : Async postfix for methods? #650

Open
StefH opened this issue Feb 22, 2019 · 5 comments
Open

ServiceBusPlugin : Async postfix for methods? #650

StefH opened this issue Feb 22, 2019 · 5 comments
Milestone

Comments

@StefH
Copy link

StefH commented Feb 22, 2019

These two methods:

public virtual Task<Message> BeforeMessageSend(Message message);
public virtual Task<Message> AfterMessageReceive(Message message);

are Async methods, so should they be postfixed by Async like?

public virtual Task<Message> BeforeMessageSendAsync(Message message);
public virtual Task<Message> AfterMessageReceiveAsync(Message message);
@SeanFeldman
Copy link
Collaborator

For consistency sake, that should be prefixed. Saying that, given this library is all about IO, the Async suffix feel somewhat unnecessary. I'd rather see Sync suffix to call out what is not asynchronous 🙂.

To address this specific issue, it would need to wait for the next major as it will be a breaking change.

@SeanFeldman SeanFeldman added this to the 4.0.0 milestone Feb 22, 2019
@axisc
Copy link

axisc commented Mar 15, 2019

Prefer to have async as the implicit/default contract, and sync as the explicit contract.

@SeanFeldman - recommend not having this change done, to avoid barriers to adopting the newest library when it is available.

Let me know if you both feel otherwise

@SeanFeldman
Copy link
Collaborator

recommend not having this change done, to avoid barriers to adopting the newest library when it is available.

Change suggested by @StefH is a good change. For the next major. I don't see how it would cause a barrier for adoption @axisc.

@axisc
Copy link

axisc commented Mar 18, 2019

I see the value, but we will have to support

public virtual Task<Message> BeforeMessageSend(Message message);
public virtual Task<Message> AfterMessageReceive(Message message);

along with

public virtual Task<Message> BeforeMessageSendAsync(Message message);
public virtual Task<Message> AfterMessageReceiveAsync(Message message);

Else, customers would have to change the references to non Async methods and add Async as a suffix.

@SeanFeldman
Copy link
Collaborator

That's API deprecation process, correct.
For v4 you'd be obsoleting with warning.
For v5 obsoleting with error.
Removing in v6.

Here's an example of how I've done this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants