Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a warning #5982

Closed

Conversation

SeanFeldman
Copy link
Contributor

No description provided.

@danielmarbach
Copy link
Contributor

@SeanFeldman thanks for going through the effort of providing this. Honestly, I'm leaning towards not merging because a warning feels like overkill, especially given we already say there

For exceptions where retrying does not make sense such as MyBusinessTimedOutException failed message is not moved to an error queue and is discarded.

@DavidBoike
Copy link
Member

I think it could be a little more clear though, see this discussion group post where this came up. Maybe we could amend the text that @danielmarbach quoted rather than add a warn?

@SeanFeldman
Copy link
Contributor Author

I agree with @DavidBoike. If it would be clear, it wouldn't be confusing and lead to misunderstanding it.

@danielmarbach
Copy link
Contributor

Amending the text I quoted would be the better solution agreed. Currently on the road and I'm unable to edit right now. Would you mind changing it @SeanFeldman?

@SeanFeldman
Copy link
Contributor Author

Anything that helps, works for me 🙂

@danielmarbach
Copy link
Contributor

@SeanFeldman I have raised an improvement to the quoted text I mentioned. The PR is available at #5991

I'll close this one for now. Thanks for the suggestion!

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