Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
On Thu, 26 Nov 2020 18:51:10 GMT, Richard Fussenegger wrote: > 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant @bridgekeeper I guess it will stay open for a little longer. - PR: https://git.openjdk.java.net/jdk/pull/1467
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
On Thu, 26 Nov 2020 18:51:10 GMT, Richard Fussenegger wrote: > 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant Active - PR: https://git.openjdk.java.net/jdk/pull/1467
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
On Fri, 15 Jan 2021 07:14:49 GMT, Richard Fussenegger wrote: >> Often an enhancement is created when there's an idea of an improvement but >> no resources to do the research and develop a justification. That work has >> to be done when someone has time. Usually ideas are discussed on one of the >> many OpenJDK email aliases to validate the idea and the approach before >> developing the code. See the list of OpenJDK Mail lists for details. >> https://mail.openjdk.java.net/mailman/listinfo >> As for moving to GitHub, it was in part to make it easier to collaborate on >> the development but also to move from Mercurial to Git. >> But GitHub does not replace the need for discussion on the Email Lists. > > Active Active - PR: https://git.openjdk.java.net/jdk/pull/1467
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
> 1. Replace ` ` with a normal space, that should work as well and is easier > to read Looks like my e-mail client was so kind and replaced the HTML character reference. It should have said: "Replace `& nbsp ;` with a normal space, ..." Additionally, if you want to search for projects using UUID.version() you can use the following CodeQL query: https://lgtm.com/query/283083268427438766/ You can (in addition to the example projects), specify custom projects to scan as well, see https://lgtm.com/help/lgtm/project-lists. Kind regards
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
The following probably does not matter much because I am not an OpenJDK contributor, but personally I think throwing an UnsupportedOperationException is reasonable: 1. It is consistent with the other methods which also only work for one specific variant 2. Code which calls UUID.version() on a non-variant 2 UUID is obviously already functionally broken; I don't think maintaining backward compatibility here adds any value Regarding the pull request, I would recommend the following changes: 1. Replace `` with a normal space, that should work as well and is easier to read 2. Add a sentence to the method description (and not only to the `@throws` tag), that this method is only meaningful for variant 2 UUIDs, see for example the documentation for `timestamp()` for how this sentence should look like to be consistent: https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/util/UUID.html#timestamp() Kind regards
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
On Thu, 17 Dec 2020 22:16:12 GMT, Roger Riggs wrote: >> I cannot create CSRs, only members can. Deprecating and switching to another >> function results in much more work for users overall because it affects all >> those who use it correctly as well. In other words: 100% >> >> I'm in favor of rejection if that's the way forward. >> >> What's wrong with creating PRs for existing issues? I thought that more >> contributions is actually the reason OpenJDK came to GitHub. > > Often an enhancement is created when there's an idea of an improvement but no > resources to do the research and develop a justification. That work has to be > done when someone has time. Usually ideas are discussed on one of the many > OpenJDK email aliases to validate the idea and the approach before developing > the code. See the list of OpenJDK Mail lists for details. > https://mail.openjdk.java.net/mailman/listinfo > As for moving to GitHub, it was in part to make it easier to collaborate on > the development but also to move from Mercurial to Git. > But GitHub does not replace the need for discussion on the Email Lists. Active - PR: https://git.openjdk.java.net/jdk/pull/1467
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
On Thu, 17 Dec 2020 22:07:36 GMT, Richard Fussenegger wrote: >>> > There is very little value in adding the exception doing so might prevent >>> > existing applications from continuing to function. >>> >>> I disagree on the value and am with the author of the original issue. All >>> these existing applications are functioning but factually invalid, adding >>> the exception like we have it in all other methods of the class would >>> surface the issue in exactly those applications. >> >> While I can appreciate that the current behavior might be less than ideal, >> changing it to throw an UnsupportedOperationException could break existing >> applications.Have you had a chance to research the number of uses of >> version() in the java eco-system to determine that the risk to existing >> applications will minimal? >> >> If you feel that the change in functionality is important for users of >> UUID, then please propose a replacement method for version and propose to >> deprecate the existing method ? >> >> If you decide to move forward with proposing a replacement method for >> version, then please socialize the proposal on corelibs-dev prior to >> creating a new PR. >> >> Please note that a CSR will also be required to move forward. > > I cannot create CSRs, only members can. Deprecating and switching to another > function results in much more work for users overall because it affects all > those who use it correctly as well. In other words: 100% > > I'm in favor of rejection if that's the way forward. > > What's wrong with creating PRs for existing issues? I thought that more > contributions is actually the reason OpenJDK came to GitHub. Often an enhancement is created when there's an idea of an improvement but no resources to do the research and develop a justification. That work has to be done when someone has time. Usually ideas are discussed on one of the many OpenJDK email aliases to validate the idea and the approach before developing the code. See the list of OpenJDK Mail lists for details. https://mail.openjdk.java.net/mailman/listinfo As for moving to GitHub, it was in part to make it easier to collaborate on the development but also to move from Mercurial to Git. But GitHub does not replace the need for discussion on the Email Lists. - PR: https://git.openjdk.java.net/jdk/pull/1467
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
On Thu, 17 Dec 2020 17:18:37 GMT, Lance Andersen wrote: >>> There is very little value in adding the exception doing so might prevent >>> existing applications from continuing to function. >> >> I disagree on the value and am with the author of the original issue. All >> these existing applications are functioning but factually invalid, adding >> the exception like we have it in all other methods of the class would >> surface the issue in exactly those applications. > >> > There is very little value in adding the exception doing so might prevent >> > existing applications from continuing to function. >> >> I disagree on the value and am with the author of the original issue. All >> these existing applications are functioning but factually invalid, adding >> the exception like we have it in all other methods of the class would >> surface the issue in exactly those applications. > > While I can appreciate that the current behavior might be less than ideal, > changing it to throw an UnsupportedOperationException could break existing > applications.Have you had a chance to research the number of uses of > version() in the java eco-system to determine that the risk to existing > applications will minimal? > > If you feel that the change in functionality is important for users of UUID, > then please propose a replacement method for version and propose to > deprecate the existing method ? > > If you decide to move forward with proposing a replacement method for > version, then please socialize the proposal on corelibs-dev prior to creating > a new PR. > > Please note that a CSR will also be required to move forward. I cannot create CSRs, only members can. Deprecating and switching to another function results in much more work for users overall because it affects all those who use it correctly as well. In other words: 100% I'm in favor of rejection if that's the way forward. What's wrong with creating PRs for existing issues? I thought that more contributions is actually the reason OpenJDK came to GitHub. - PR: https://git.openjdk.java.net/jdk/pull/1467
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
On Wed, 16 Dec 2020 14:42:36 GMT, Richard Fussenegger wrote: > > There is very little value in adding the exception doing so might prevent > > existing applications from continuing to function. > > I disagree on the value and am with the author of the original issue. All > these existing applications are functioning but factually invalid, adding the > exception like we have it in all other methods of the class would surface the > issue in exactly those applications. While I can appreciate that the current behavior might be less than ideal, changing it to throw an UnsupportedOperationException could break existing applications.Have you had a chance to research the number of uses of version() in the java eco-system to determine that the risk to existing applications will minimal? If you feel that the change in functionality is important for users of UUID, then please propose a replacement method for version and propose to deprecate the existing method ? If you decide to move forward with proposing a replacement method for version, then please socialize the proposal on corelibs-dev prior to creating a new PR. Please note that a CSR will also be required to move forward. - PR: https://git.openjdk.java.net/jdk/pull/1467
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
On Mon, 14 Dec 2020 19:36:45 GMT, Roger Riggs wrote: > There is very little value in adding the exception doing so might prevent > existing applications from continuing to function. I disagree on the value and am with the author of the original issue. All these existing applications are functioning but factually invalid, adding the exception like we have it in all other methods of the class would surface the issue in exactly those applications. - PR: https://git.openjdk.java.net/jdk/pull/1467
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
On Thu, 26 Nov 2020 18:51:10 GMT, Richard Fussenegger wrote: > 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant The `version` method is described in the javadoc as only being applicable to variant = 2 UUIDs. Changing the behavior to throw a `UnsupportedOperationException` is not a compatible change. There is very little value in adding the exception doing so might prevent existing applications from continuing to function. Repeating the limitation to version 2 to the javadoc of the version method as far as this request can go. Otherwise, please close the PR without integrating. - Changes requested by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1467
RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
6594730: UUID.getVersion() is only meaningful for Leach-Salz variant - Commit messages: - 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant Changes: https://git.openjdk.java.net/jdk/pull/1467/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1467=00 Issue: https://bugs.openjdk.java.net/browse/JDK-6594730 Stats: 16 lines in 2 files changed: 14 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1467.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1467/head:pull/1467 PR: https://git.openjdk.java.net/jdk/pull/1467
