-
Notifications
You must be signed in to change notification settings - Fork 542
8252546: Move ObservableValue's equality check and lazy evaluation descriptions to @implSpec #292
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
|
/csr |
|
👋 Welcome back nlisker! A progress list of the required criteria for merging this PR into |
|
@nlisker has indicated that a compatibility and specification (CSR) request is needed for this pull request. |
Webrevs
|
|
Once we settle on the docs I'll create the CSR. |
kevinrushforth
left a comment
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.
I left a couple suggestions below.
modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java
Outdated
Show resolved
Hide resolved
modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java
Outdated
Show resolved
Hide resolved
modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java
Outdated
Show resolved
Hide resolved
kevinrushforth
left a comment
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.
Looks good with one minor comment. Go ahead and create the CSR once you make the requested update.
modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java
Outdated
Show resolved
Hide resolved
|
@nlisker This change now passes all automated pre-integration checks. In addition to the automated checks, the change must also fulfill all project specific requirements After integration, the commit message will be:
Since the source branch of this PR was last updated there have been 8 commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge ➡️ To integrate this PR with the above commit message to the |
|
/reviewers 2 |
|
@kevinrushforth |
|
I see that SKARA-548 is still an issue, and that the PR was marked ready without the CSR being approved. |
|
Yeah I saw. The CSR has been submitted. I think that the bot isn't picking up on it even. |
|
It's not really ready to be integrated, right? It's not finding the CSR I think. |
|
Correct. It is not yet ready. SKARA-548 is not yet resolved, so shows as ready even though it isn't. |
|
The CSR has been approved, I'll go ahead and integrate. |
|
/integrate |
|
@nlisker Since your change was applied there have been 8 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit b2e2000. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Moving implementation details about lazy evaluation and equality checking to
@implSpec.Progress
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/jfx pull/292/head:pull/292$ git checkout pull/292