Remove Eclipse annotations package. #6992
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sovdeeth
approved these changes
Aug 31, 2024
Moderocky
added
the
patch-ready
APickledWalrus
approved these changes
Sep 4, 2024
There was a problem hiding this comment.
The idea is fine. The other formatting changes (e.g. whitespace-only lines converted to blank) make me a little nervous for conflicts as GitHub has a tendency to complain, but it's not a big deal. I wonder if it would be worth doing away with the license headers at this point too?
Moderocky
force-pushed
the
bye-bye-annotations
branch
from
9d32012 to
4c1441a
Compare
Nuutrai
pushed a commit
to Nuutrai/Skript
that referenced
this pull request
Sep 19, 2024
* Remove dependencies. * Remove whitespace changes. * Revert branch merge error.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We are repeatedly having to request changes on PRs to change the annotations.
Although this saves effort (in terms of resolving conflicts) I think it is costing time in the long run because it necessitates more reviews and re-reviews.
This PR does all the changes at once, meaning any future PR will be unaffected (and any existing PR can just merge the changes if they haven't already).
I used an automatic script for it, so it's possible that the odd mistake exists, but since it builds and tests correctly, I would expect it only to be in documentation if at all.
This also removes whitespace in a number of large legacy files according to the contribution guidelines:
The whitespace causes a number of changes in the file diff of PRs that aren't actually related to the PR and simply take up space making it harder to review. I wanted to squash a lot of it in one go so we won't have to deal with it later.
Description
Removes the
org.eclipse.jdtlibrary responsible for eclipse annotations.Replaces all existing Eclipse annotations with the JetBrains alternative (e.g. NonNull -> NotNull).
This also removes the package-level eclipse annotations that were causing incorrect nullability warnings for addon developers in some place and encouraging incorrect assumptions (see the discussion re: 6684).
Related Issues: #6684