-
Notifications
You must be signed in to change notification settings - Fork 292
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 the @discardableResult attribute to OrderedSet.insert(_:at:) #21
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, thank you!
It would be useful to check if there any other APIs in the package that are currently missing this attribute -- if we missed one, we may have missed others.
@swift-ci test |
I did wonder if the following functions should have this attribute, too: |
@kielgillard I think so, if only by analogue to (I wonder how often these would be used without looking at the return value, but there is nothing wrong with discarding it, either -- and I don't think we want to force people to do the |
@swift-ci test |
@lorentey done. My intuition is people will probably be using these "update or" APIs specifically for the return value but, like you said, there's no need for the API to be a jerk about it in case you don't want it! |
Looks good, thanks! 👍 @swift-ci test |
Fixes #19.
It does not seem appropriate to write a test for this change. However, I am open to other points of view and will gladly own further work.
Checklist