Conversation
eirikbakke
left a comment
There was a problem hiding this comment.
Looks good, only question is whether we should also remove hashCode() from the two hashmap key classes that became records.
- simplify if/else image loading cascade - remove redundant logger with same name - java 17 language level upgrade, dead code removal etc
d7d2132 to
e967887
Compare
|
@eirikbakke btw while testing I noticed those log lines: seems to come from the form designer palette window. the utility only accounts for netbeans/platform/openide.util.ui/src/org/openide/util/ImageUtilities.java Lines 234 to 243 in c384b9a |
|
checked a few things and everything worked -> merging thanks for review @eirikbakke! |
|
Nice catch regarding nbresloc vs. nbres! We should handle both. There is an explanation of the difference here: Apparently, nbresloc should use localization while nbres should not. So they should be handled the same except with a different second argument to loadImage(String,boolean). Perhaps you could just add this as part of this PR? Maybe also update the Javadoc of loadImage(URI) to refer to say "nbres or nbresloc protocols" instead of just "nbresloc". |
|
@eirikbakke oops I didn't see your message before merging. Lets add this in a separate PR to not mix cleanup with other changes. |
|
Yeah, sure. I can make the PR and request your review. |
to shrink my stash a bit :)
Originally I only intended to simplify the loading method and remove the logger - but there will be more eyes on this class due to all the usage refactoring so why not do a more complete cleanup. This might be easier to review in the IDE side-by-side view.