-
Notifications
You must be signed in to change notification settings - Fork 901
Remove unused/deprecated openide.options module #8755
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
Conversation
dfedfba
to
3ac9b1d
Compare
3ac9b1d
to
bbd1b4e
Compare
# List of old and deprecated APIs for building complete javadoc | ||
config.javadoc.deprecated=\ | ||
openide.options | ||
config.javadoc.deprecated= |
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.
had to update the code in nbbuild/antsrc/org/netbeans/nbbuild/SetApidocClustersConfig.java
since it expected an non-empty list
Looks good. Maybe duplicate the title into the body so that it's not empty? |
|
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.
- Keep the module for backward compatibility reasons
- at least that's what I'd do to follow the original NetBeans Platform compatibility visions.
- it just lies around causing no harm
@jtulach full backwards compatibility is a nice ideal, but it's certainly not been a reality with the platform in at least the 15+ years I've worked with it. It should not be used to make things unnecessarily more difficult. This is going to come up more and more over the coming year or two as the ecosystem we rely on moves on while various features of the IDE and platform are lying around without getting necessary updates. It's not true to say this just causes no harm - see the linked issues eg. #8256 If you wish to propose changes to this module to meet those issues instead, then feel free if you have the time and inclination - because I'm not sure anyone else does.
older versions up to NB 27 of this module are on maven central and the deprecation happened 19y ago with the commit msg: "Marking module deprecated to remind people not to use it.". Stopping to release it is just the next step to remind people to not use it.
I disagree. Dead code has cumulative maintenance costs associated with it. Security team has to respond to exploit notifications and javac, code scanners and vulnerability scanners produce extra noise which is in the way every time a real issue is investigated. And as mentioned, the module uses JDK API which is deprecated for removal. We shouldn't wait for things to break before we have to react in a hurry, we pushed our luck with the security manager removal once, we don't have to do this every time. |
rebasing and running all tests before integration. edit: all green except javadoc link checker, will try to find the reference I overlooked |
bbd1b4e
to
a127d7e
Compare
6c7af2b
to
6dd9890
Compare
- module isn't used by NB itself and was already deprecated before NB was donated to apache (19y ago) - uses java.beans.beancontext which is deprecated-for-removal since JDK 23 - it is still shipped in the zip and maven artifacts generating warnings - even though its not in classpath at runtime, it is still hooked into the test class path across the project. Shrinking the cp helps to avoid windows specific arg length limits CI hit a few times already.
6dd9890
to
8b5d968
Compare
all green, made only minor updates to resolve the javadoc link check problems since approval. Checked the thread on the private list and it had no new comments regarding this PR -> merging. thanks for the reviews! |
java.beans.beancontext
which is deprecated-for-removal since JDK 23meta issue #8256