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

Implement new JDK21 SortedSet and List methods for SortedList #6134

Merged
merged 1 commit into from
May 31, 2024

Conversation

neeshy
Copy link
Contributor

@neeshy neeshy commented May 28, 2024

As per #6131

Copy link
Contributor

@kriegfrj kriegfrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update, @neeshy ! Looks like the compile fails because on Java 17 these methods do not exist. I think that we still need to preserve Java 17 compile compatibility. So perhaps a solution here will be to comment out the @OverRide line.

I suspect that this change will also require you to do a minor version update in package-info.java from 4.2.0 to 4.3.0:

https://github.com/bndtools/bnd/blob/master/aQute.libg/src/aQute/lib/collections/package-info.java

The build never got that far because of the compile error though.

@neeshy
Copy link
Contributor Author

neeshy commented May 28, 2024

I've removed the @Override lines and it seem to build fine without the version update in package-info.java.

@chrisrueger
Copy link
Contributor

chrisrueger commented May 28, 2024

it seem to build fine without the version update in package-info.java.

Unfortunately not.
See failing build https://github.com/bndtools/bnd/actions/runs/9265278739/job/25492215091?pr=6134

aQute/lib/collections/package-info.java:0: error: Baseline mismatch for package aQute.lib.collections, MINOR change. Current is 4.2.0, repo is 4.2.0, suggest 4.3.0 or -

MINOR                PACKAGE    aQute.lib.collections
  MINOR              CLASS      aQute.lib.collections.SortedList
    ADDED            METHOD     addFirst(java.lang.Object)
      ADDED          RETURN     void
    ADDED            METHOD     addLast(java.lang.Object)
      ADDED          RETURN     void
    ADDED            METHOD     getFirst()
      ADDED          RETURN     java.lang.Object
    ADDED            METHOD     getLast()
      ADDED          RETURN     java.lang.Object
    ADDED            METHOD     removeFirst()
      ADDED          RETURN     java.lang.Object
    ADDED            METHOD     removeLast()
> Task :aQute.libg:jar FAILED
      ADDED          RETURN     java.lang.Object
    ADDED            METHOD     reversed()
      ADDED          RETURN     aQute.lib.collections.SortedList

65 actionable tasks: 65 executed
FAILURE: Build failed with an exception.

@kriegfrj
Copy link
Contributor

I've removed the @Override lines and it seem to build fine without the version update in package-info.java.

Thanks for the update!

It will compile ok, but the baselines check (which happens later in the build) will fail - as it has done with the last push you submitted. I think if you update the version to 4.3.0 in package-info, this will fix the build error.

@kriegfrj
Copy link
Contributor

Also, when you re-submit this fix, can you please massage your commit message to include "Fixes #6131" on the bottom line, so that the full message looks something like this:

[libg] Implement new JDK21 SortedSet and List methods for SortedList

Fixes #6131

That way we can use the commit history to refer back to all of this discussion for historical context if necessary, and as an added bonus when we merge it it will automatically close the associated issue. Adding the [libg] at the front, while not as important or enforced, is something that I have personally done in the past as it also allows us to see at a glance which part of the workspace the commit is for.

@neeshy
Copy link
Contributor Author

neeshy commented May 29, 2024

Ah, I was building it through my package manager which evidently doesn't run that check. Pushed with the suggested changes.

@kriegfrj kriegfrj merged commit 70612e3 into bndtools:master May 31, 2024
9 checks passed
@kriegfrj
Copy link
Contributor

Thank you so much, @neeshy ! Congratulations on earning your honorary "first contribution" badge!

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.

Compile error on JDK 21: aQute/lib/collections/SortedList doesn't fully implement SortedSet/List interfaces
3 participants