Skip to content

Conversation

@Korov
Copy link

@Korov Korov commented Jul 29, 2023

fix JDK-8305733


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • JDK-8214245 needs maintainer approval
  • Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)
  • Change requires CSR request JDK-8328951 to be approved

Issues

  • JDK-8214245: Case insensitive matching doesn't work correctly for some character classes (Bug - P4)
  • JDK-8328951: Case insensitive matching doesn't work correctly for some character classes (CSR)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev.git pull/2062/head:pull/2062
$ git checkout pull/2062

Update a local copy of the PR:
$ git checkout pull/2062
$ git pull https://git.openjdk.org/jdk11u-dev.git pull/2062/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2062

View PR using the GUI difftool:
$ git pr show -t 2062

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/2062.diff

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Jul 29, 2023

👋 Welcome back Korov! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr Pull request is ready for review label Jul 29, 2023
@mlbridge
Copy link

mlbridge bot commented Jul 29, 2023

@openjdk openjdk bot removed the rfr Pull request is ready for review label Jul 30, 2023
@openjdk openjdk bot added the rfr Pull request is ready for review label Jul 31, 2023
@openjdk
Copy link

openjdk bot commented Aug 10, 2023

@Korov This change is no longer ready for integration - check the PR body for details.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Aug 10, 2023
@Korov
Copy link
Author

Korov commented Aug 11, 2023

/integrate

@openjdk openjdk bot added the sponsor Pull request is ready to be sponsored label Aug 11, 2023
@openjdk
Copy link

openjdk bot commented Aug 11, 2023

@Korov
Your change (at version f7efe2a) is now ready to be sponsored by a Committer.

@TheRealMDoerr
Copy link
Contributor

This repository requires approval before integration. See https://wiki.openjdk.org/display/JDKUpdates/How+to+contribute+or+backport+a+fix step 6.

@phohensee
Copy link
Member

@Korov, what testing have you done, and what do you estimate the risk is?

@Korov
Copy link
Author

Korov commented Aug 21, 2023

@Korov, what testing have you done, and what do you estimate the risk is?

Here we test whether the matching of Posix character, Unicode character, Unicode categories, aliases, properties, and java methods is correct under the condition of ignoring case.

Currently considered no risk.

@phohensee
Copy link
Member

@Korov, please post a fix request comment for the JBS issue outlining risk and testing.

@phohensee
Copy link
Member

Tagged the JBS issue.

@RealCLanger
Copy link
Contributor

I don't get this. What is the difference of this change to a full backport of JDK-8214245? If there's no difference, this should be a backport of JDK-8214245. And then we'd also need to file a CSR.

@Korov
Copy link
Author

Korov commented Sep 1, 2023

@RealCLanger There's no difference, But I have no permission to login JBS, and I cannot create a CSR from https://bugreport.java.com/bugreport/, Can anyone give me some help on how to create a CSR? Or should I close this PR?

@Korov
Copy link
Author

Korov commented Sep 1, 2023

After learning about CSR, does it mean that this change may cause changes in the behavior of some programs, causing compatibility issues?

@openjdk openjdk bot removed sponsor Pull request is ready to be sponsored ready Pull request is ready to be integrated labels Sep 5, 2023
@RealCLanger
Copy link
Contributor

RealCLanger commented Sep 5, 2023

@RealCLanger Thank you very much for your help, I have changed the title.

Looks like the bots don't recognize the short hash. You need to rename it "Backport 1d4a4fed437184b2f9fef605de40aab82846c717" (which is what I wanted to say but GitHub made the short hash out of it). And also remove the "8305733:" from the title.

@Korov Korov changed the title 8305733: Backport 1d4a4fe 8305733: Backport 1d4a4fed437184b2f9fef605de40aab82846c717 Sep 6, 2023
@Korov Korov changed the title 8305733: Backport 1d4a4fed437184b2f9fef605de40aab82846c717 Backport 1d4a4fed437184b2f9fef605de40aab82846c717 Sep 6, 2023
@openjdk openjdk bot changed the title Backport 1d4a4fed437184b2f9fef605de40aab82846c717 8214245: Case insensitive matching doesn't work correctly for some character classes Sep 6, 2023
@openjdk
Copy link

openjdk bot commented Sep 6, 2023

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added backport Port of a pull request already in a different code base sponsor Pull request is ready to be sponsored ready Pull request is ready to be integrated labels Sep 6, 2023
@GoeLin
Copy link
Member

GoeLin commented Sep 22, 2023

/reviewers 2

Asking for a second review because intergrate was already triggered, but the CSR is still missing.

@openjdk
Copy link

openjdk bot commented Sep 22, 2023

@GoeLin
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

@openjdk openjdk bot removed sponsor Pull request is ready to be sponsored ready Pull request is ready to be integrated labels Sep 22, 2023
@bridgekeeper
Copy link

bridgekeeper bot commented Oct 20, 2023

@Korov This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

@RealCLanger
Copy link
Contributor

@Korov Sorry for being inactive on this one for so long. Are you still willing to follow up on this backport? In that case I would create the CSR now. Please let me know.

@Korov
Copy link
Author

Korov commented Nov 1, 2023

@RealCLanger Yes

@bridgekeeper
Copy link

bridgekeeper bot commented Nov 29, 2023

@Korov This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

@bridgekeeper
Copy link

bridgekeeper bot commented Dec 27, 2023

@Korov This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

@bridgekeeper bridgekeeper bot closed this Dec 27, 2023
@Korov
Copy link
Author

Korov commented Mar 23, 2024

/open

@openjdk openjdk bot reopened this Mar 23, 2024
@openjdk
Copy link

openjdk bot commented Mar 23, 2024

@Korov This pull request is now open

@Korov
Copy link
Author

Korov commented Mar 23, 2024

Hello @RealCLanger , I'm very interested in continuing to work on this backport, Please help me create a CSR and tell me the relevant links. Thank you very much for your help.

@openjdk openjdk bot added the csr Pull request needs approved CSR before integration label Mar 25, 2024
@RealCLanger
Copy link
Contributor

Hello @RealCLanger , I'm very interested in continuing to work on this backport, Please help me create a CSR and tell me the relevant links. Thank you very much for your help.

OK, I now created a CSR for 11-pool: JDK-8328951. The CSR will need a review from some OpenJDK reviewer, e.g. @phohensee or @GoeLin, would you mind taking a look?

After CSR review, we have to wait whether it'll be approved before we can continue with this backport.

@Korov
Copy link
Author

Korov commented Mar 26, 2024

Hello @RealCLanger , I'm very interested in continuing to work on this backport, Please help me create a CSR and tell me the relevant links. Thank you very much for your help.

OK, I now created a CSR for 11-pool: JDK-8328951. The CSR will need a review from some OpenJDK reviewer, e.g. @phohensee or @GoeLin, would you mind taking a look?

After CSR review, we have to wait whether it'll be approved before we can continue with this backport.

Thats great, thanks for your help. I will continue to pay attention to the progress of CSR.

@RealCLanger
Copy link
Contributor

@Korov The CSR request got rejected, please see the comment. I agree that this change bears a compatibility risk that is maybe too high for a backport.

What's the reason that you need this in JDK11? E.g. would it be possible for you to move on to JDK17?

@Korov
Copy link
Author

Korov commented Apr 13, 2024

@Korov The CSR request got rejected, please see the comment. I agree that this change bears a compatibility risk that is maybe too high for a backport.

What's the reason that you need this in JDK11? E.g. would it be possible for you to move on to JDK17?

This has been fixed in JDK17, so I think this PR can be closed.

@Korov Korov closed this Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Port of a pull request already in a different code base csr Pull request needs approved CSR before integration rfr Pull request is ready for review

Development

Successfully merging this pull request may close these issues.

5 participants