-
Notifications
You must be signed in to change notification settings - Fork 317
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
Added IterableExtensions.lastOrNull and deprecated last #2983
Conversation
gwt-maven plugin: The attribute since is undefined for the annotation type Deprecated
@@ -87,7 +87,7 @@ protected Integer findLast(Iterable<Integer> input, Function1<Integer, Boolean> | |||
|
|||
@Override | |||
protected Integer last(Iterable<Integer> input) { |
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.
Can you please rename this method last
also to lastOrNull
?
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.
@szarnekow this method is defined as abstract in the superclass org.eclipse.xtext.xbase.tests.lib.BaseIterablesIteratorsTest.last(IterableOrIterator)
so I should rename it also in IteratorExtensionsTest
.
I changed to lastOrNull
only in IterableExtensions
, NOT in IteratorExtensions
.
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.
Yes, I saw that. I wonder if we should both keep in symmetry, though. Thoughts?
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.
I also thought about that, but how do we "justify" the deprecation for IteratorExtensions
? Of course, we could use the "symmetry" justification.
Also head
could be changed accordingly.
Unfortunately, we are forced to change IterableExtensions.last
only for the clash with Java 21 new API.
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.
So far I liked that the extension API for iterables and iterators was equivalent (as far as it goes).
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.
Ok, then I'll add lastOrNull to IteratorExtensions and update this PR
@szarnekow I've added when the build is green, I'll merge |
Closes #2981
I have updated the occurrences in our code base, but NOT in the input files of our tests.