-
Notifications
You must be signed in to change notification settings - Fork 99
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
Replace usage of internal SWT TypedListener and SWTEventListener #583
Replace usage of internal SWT TypedListener and SWTEventListener #583
Conversation
Yes it will at least require a new SWT release |
Yes. I also want to mention that, even if SWT will eventually deprecate and (re-)move the |
@@ -33,7 +33,7 @@ Contributors: | |||
<jacoco-version>0.8.9</jacoco-version> | |||
<easymock-version>5.2.0</easymock-version> | |||
|
|||
<target-platform>https://download.eclipse.org/releases/latest</target-platform> | |||
<target-platform>https://download.eclipse.org/eclipse/updates/4.32-I-builds/</target-platform> |
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.
FYI, replacing the release train site with the Platform's I-builds site is going to reduce the available set of 3rd party bundles significantly, leading to resolution failures.
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 noticed that as well. But since I assumed it will take some time until this is submitted anyway and it was temporary I was lazy to look up how the repos are set up.
But now I checked it and it should work. :)
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.
Now the build succeeded :)
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.
After 4.32 M1 release you could change it to https://download.eclipse.org/releases/2024-06/
?
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 that would be possible too, but would then later have to be changed back, if the latest is generally desired.
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.
FYI, during the end of the last release cycle SimRel produced and now maintains this:
https://download.eclipse.org/releases/milestone/
as requested here:
eclipse-simrel/simrel.build#286
This site has the advantage that it tracks milestones and release candidates so that any problems are noticed before the release rather than after the release.
4ad2ebb
to
db4818a
Compare
<repository> | ||
<id>eclipse-iBuild</id> | ||
<layout>p2</layout> | ||
<url>https://download.eclipse.org/eclipse/updates/4.32-I-builds</url> | ||
</repository> |
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.
This should be reverted before submission.
Thank you @HannesWell . It seems to be a big change. @wimjongman what do you think about this proposal ? |
...org.eclipse.nebula.widgets.cdatetime/src/org/eclipse/nebula/widgets/cdatetime/CDateTime.java
Outdated
Show resolved
Hide resolved
db4818a
to
762f04e
Compare
When the method is officially released, we can take it. |
762f04e
to
c280306
Compare
c280306
to
27d6297
Compare
It was just released today. I rebased the branch and resolved all conflicts. |
Leverage new methods introduced in eclipse-platform/eclipse.platform.swt#1112
27d6297
to
e530c60
Compare
Any objections to merging this now? |
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.
Some queries.
...el/org.eclipse.nebula.widgets.carousel/src/org/eclipse/nebula/widgets/carousel/Carousel.java
Outdated
Show resolved
Hide resolved
...e.nebula.widgets.calendarcombo/src/org/eclipse/nebula/widgets/calendarcombo/CustomCombo.java
Show resolved
Hide resolved
Thanks, Hannes! |
Thanks for your review! |
The
org.eclipse.swt.widgets.TypedListener
should be considered an internal class (as the doc states) and leaks the classSWTEventListener
which also resides in an internal package.Leverage new methods introduced in eclipse-platform/eclipse.platform.swt#1112 in order to avoid the usage of that two classes. I don't know if there is a specific backwards compatibility policy in Nebula regarding SWT, but at the moment the new methods are only available in Eclipse I-builds. I guess therefore this change cannot be applied immediately.
In order to demonstrate it works I changed temporarily changed the target-platform of the build.
@lcaron could you please have a look at this one?
@Phillipus for your information (if you want to review this too, please do so).