Skip to content

Conversation

@merlinran
Copy link
Contributor

@merlinran merlinran commented Jan 18, 2023

It adds one commit on top of #143 to add a PublishNoBlock method. It's useful in my use case, but I'm not sure how others would think, so feel free to reject it!

@merlinran merlinran force-pushed the non-blocking-publish branch 3 times, most recently from 2372042 to bb4c7ad Compare January 18, 2023 12:22
@purehyperbole
Copy link
Member

Hey @merlinran thanks again for another contribution!

Definitely looks like a worthwhile addition. However, I feel naming that function something more descriptive to indicate that it can fail to send the message might be clearer. i.e.

  • PublishOrDiscard(id string, event *Event)
  • AttemptPublish(id string, event *Event)
  • TryPublish(id string, event *Event)

Also returning a true or false depending on a success or failure may be useful for the caller.

@merlinran merlinran force-pushed the non-blocking-publish branch from bb4c7ad to b2cb983 Compare January 18, 2023 12:34
@merlinran
Copy link
Contributor Author

I like TryPublish! Also good idea on returning a bool! Updated.

@purehyperbole
Copy link
Member

Awesome, thanks again. I will merge this and create a new release with the other change 👍

@purehyperbole purehyperbole merged commit c6d5381 into r3labs:master Jan 18, 2023
@merlinran merlinran deleted the non-blocking-publish branch January 18, 2023 13:04
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.

2 participants