You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by kinow <gi...@git.apache.org> on 2017/07/09 10:08:03 UTC
[GitHub] commons-lang pull request #275: [WIP] LANG-1339: replace java.beans.Property...
GitHub user kinow opened a pull request:
https://github.com/apache/commons-lang/pull/275
[WIP] LANG-1339: replace java.beans.PropertyListener by java.util.Observable
For discussion, regarding [LANG-1339](https://issues.apache.org/jira/browse/LANG-1339) (TL;DR: remove depedency on java.desktop from [lang], preparing for Java 9). Thought having some code to discuss would be helpful.
This pull request contains a possible solution, where java.beans.{PropertyChangeListener,PropertyChangeListener} are replaced by java.util.{Observer,Observable}.
Users would still have the same feature, but alas binary compatibility would not be kept. So perhaps this could be in a 4.x release. The other methods are deprecated in the next 3.x releases, and we remove the dependency to java.desktop on 4.x, and also offer a Java9 module with no dependency java.desktop.
Would this be a viable alternative?
Cheers
Bruno
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kinow/commons-lang LANG-1339
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/commons-lang/pull/275.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #275
----
commit 0ba4ea983b0e2ffdffd102b23921292c3c92d3ac
Author: Bruno P. Kinoshita <ki...@apache.org>
Date: 2017-07-09T10:01:06Z
LANG-1339: replace java.beans.PropertyListener by java.util.Observable
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang issue #275: [WIP] LANG-1339: replace java.beans.PropertyListene...
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/275
@jodastephen done. Kept the existing classes, but added new ones where the only modification is replacing `java.beans` by `java.util` equivalent classes.
Existing classes were annotated with `@deprecated` with a comment pointing to the new class.
WDTY? I'd like to sort it out as soon as possible to sort out the issue with dependencies & Java 9 in lang.
Thanks!
Bruno
ps: old code was removed from commit line. Moved instead to a branch at https://github.com/kinow/commons-lang/tree/LANG-1339-old, just in case we need to compare it or someone wants to see what it was before
---
[GitHub] commons-lang issue #275: LANG-1339: replace java.beans.PropertyListener by j...
Posted by PascalSchumacher <gi...@git.apache.org>.
Github user PascalSchumacher commented on the issue:
https://github.com/apache/commons-lang/pull/275
+1
---
[GitHub] commons-lang issue #275: [WIP] LANG-1339: replace java.beans.PropertyListene...
Posted by jodastephen <gi...@git.apache.org>.
Github user jodastephen commented on the issue:
https://github.com/apache/commons-lang/pull/275
The PR is indeed backwards incompatible.
My suggestion would be to add two new classes with the fixed code (different class names), and deprecated the old classes. That way there is no need to have commons-lang v4.
---
[GitHub] commons-lang issue #275: [WIP] LANG-1339: replace java.beans.PropertyListene...
Posted by britter <gi...@git.apache.org>.
Github user britter commented on the issue:
https://github.com/apache/commons-lang/pull/275
Removing the dependency in 4.x is the way to go. I don't see a way to get this into 3.x :-(
---
[GitHub] commons-lang issue #275: [WIP] LANG-1339: replace java.beans.PropertyListene...
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/275
>My suggestion would be to add two new classes with the fixed code (different class names), and deprecated the old classes. That way there is no need to have commons-lang v4.
Thought a bit about that after seeing the vote for [lang] 3.7. In any way, the current implementation will be removed only in 4.x, and we will have to include the require static for Java 9 module.
So right now I am thinking about not marking the class or fields as deprecated, nor adding another class with a different name. Just keep this PR and the ticket open. Then mark the ticket as FixVersion 4.0, use the require static trick for the module-info.java for Java 9.
Does that sound like a good plan?
---
[GitHub] commons-lang issue #275: [WIP] LANG-1339: replace java.beans.PropertyListene...
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/275
Yup @britter you are definitely correct. At least if we agree on the solution, we can think about marking some methods as deprecated in the current circuit breaker (though I prefer to mark as deprecated when there is an alternative), and/or add notes in the next release notes, alerting users about the change to come.
---
[GitHub] commons-lang issue #275: [WIP] LANG-1339: replace java.beans.PropertyListene...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/275
[](https://coveralls.io/builds/17405129)
Coverage increased (+0.03%) to 95.271% when pulling **e089e4c294292f8255701429ec9b22057025a1a8 on kinow:LANG-1339** into **70be8e5148a2f616399c3205c169df600400833c on apache:master**.
---
[GitHub] commons-lang issue #275: WIP LANG-1339: replace java.beans.PropertyListener ...
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/275
Back to WIP. Please avoid merging it for now. Gilles noted in the mailing list that the Observable/Observer classes have been deprecated in Java 9. So we will probably go with the deprecation/removal path instead.
Link to the discussion in the mailing list: https://markmail.org/message/efy52lul5pzbznut
---
[GitHub] commons-lang issue #275: LANG-1339: replace java.beans.PropertyListener by j...
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/275
Posted to the mailing list and will wait to see if there is any further feedback. Otherwise I will merge it in the next days. Thank you @jodastephen !
---
[GitHub] commons-lang issue #275: [WIP] LANG-1339: replace java.beans.PropertyListene...
Posted by jodastephen <gi...@git.apache.org>.
Github user jodastephen commented on the issue:
https://github.com/apache/commons-lang/pull/275
Thanks @kinow , this seems like the right solution. Now the module-info can just use `require static` to avoid creating a full dependency on the awkward three classes and users have a practical alternative (either add the `java.desktop` dependency manually, or migrate to the replacement classes.
---