-
Notifications
You must be signed in to change notification settings - Fork 580
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 DeliveryModes convenience enum #1063
Conversation
Thank you. Besides a bit of work on naming, this looks like a good addition to me. |
How about an enum? public enum DeliveryMode : byte
{
Transient = 1,
Persistent = 2
} |
I like @Matthew-Davey's idea. @Jalalx WDYT about using an enum? |
Using an
That's why I considered going with simple Please let me know what you think. |
I'd agree with the above statement of @Jalalx . On the other hand I'm not sure how the usage is divided between the DeliveryMode and the Persistent property. I've personally never used the DeliveryMode. So the benefit for me would be 0. |
Yes, but I wasn't aware of that property until I see that reference while making this PR 😅 I guess |
But it's acceptable for the next major version. See #1056 |
@sungam3r Does it worth a breaking change? What do you think @michaelklishin? |
Edit: whooops, didn't read the whole comment section, my bad. disregard my comment. |
Why not if v7 is on the way? Nobody likes magic numbers. |
So, should I drop the |
ee1ca49
to
5f7adba
Compare
ca2bf39
to
a93d8b1
Compare
change NonPersistent const name in DeliveryModes to Transient Update DeliveryMode to be an enum
a93d8b1
to
99ea72e
Compare
Thank you @Jalalx |
Proposed Changes
This pull request adds the
DeliveryModes
convenience class as a helper to prevent developers from hardcoding delivery mode codes using magic numbers.Types of Changes
What types of changes does your code introduce to this project?
Checklist
CONTRIBUTING.md
document