Skip to content
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

8312165: Fix typos in java.desktop Swing #14847

Closed
wants to merge 9 commits into from

Conversation

turbanoff
Copy link
Member

@turbanoff turbanoff commented Jul 12, 2023

Found many typos in java.desktop by IDEA's inspection Proofreading | Typo


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue

Issue

  • JDK-8312165: Fix typos in java.desktop Swing (Bug - P5)

Reviewers

Contributors

  • Alexey Ivanov <aivanov@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/14847/head:pull/14847
$ git checkout pull/14847

Update a local copy of the PR:
$ git checkout pull/14847
$ git pull https://git.openjdk.org/jdk.git pull/14847/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 14847

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/14847.diff

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Jul 12, 2023

👋 Welcome back aturbanov! 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
Copy link

openjdk bot commented Jul 12, 2023

@turbanoff The following labels will be automatically applied to this pull request:

  • client
  • i18n

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added client client-libs-dev@openjdk.org i18n i18n-dev@openjdk.org labels Jul 12, 2023
@turbanoff turbanoff changed the title [PATCH] Fix typos in java.desktop 8312165: Fix typos in java.desktop Jul 17, 2023
@openjdk openjdk bot added the rfr Pull request is ready for review label Jul 17, 2023
@mlbridge
Copy link

mlbridge bot commented Jul 17, 2023

Webrevs

@bridgekeeper
Copy link

bridgekeeper bot commented Aug 14, 2023

@turbanoff 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!

@turbanoff
Copy link
Member Author

Keep alive. Let's wait for review a bit longer

Copy link
Member

@aivanov-jdk aivanov-jdk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please break such changes into smaller batches: reviewing nearly 200 files is tedious and requires lots of time. Changesets consisting of no more than 20 or 40 files would likely be reviewed quicker.

revert changes from non-swing parts to minimize count of files to review
apply suggestions from review
@turbanoff turbanoff changed the title 8312165: Fix typos in java.desktop 8312165: Fix typos in java.desktop Swing Aug 30, 2023
@turbanoff
Copy link
Member Author

turbanoff commented Aug 30, 2023

I reverted changes in non-Swing parts to reduce amount of files to review.
Merged with master to fix GHA builds.

@aivanov-jdk
Copy link
Member

I reverted changes in non-Swing parts to reduce amount of files to review.

I'm unsure it's the right decision, I reviewed them already. On the other hand, 100 files is a smaller chunk, someone could also take a look.

Thanks for fixing typos, by the way.

apply suggestions from review
Copy link
Member

@aivanov-jdk aivanov-jdk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second “short-circuit” in JLayeredPane at line 558 is still with the space:

https://github.com/openjdk/jdk/pull/14847/files#diff-ccb2c8aa4e9d637576d0e386625f6ec0f3fc04f481277f994af23542ecc11d7fL558

even though you marked the comment as resolved.

apply suggestions from review
@turbanoff
Copy link
Member Author

/contributor add @aivanov-jdk

@turbanoff
Copy link
Member Author

even though you marked the comment as resolved.

Ooops. Sorry for that. Somehow I forgot to commit changes in this file.

@openjdk
Copy link

openjdk bot commented Sep 1, 2023

@turbanoff
Contributor Alexey Ivanov <aivanov@openjdk.org> successfully added.

Copy link
Member

@aivanov-jdk aivanov-jdk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me except for the minor comment.

@openjdk
Copy link

openjdk bot commented Sep 1, 2023

@turbanoff This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8312165: Fix typos in java.desktop Swing

Co-authored-by: Alexey Ivanov <aivanov@openjdk.org>
Reviewed-by: aivanov

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 46 new commits pushed to the master branch:

  • 2f7c65e: 8303427: Fixpath confused if unix root contains "/jdk"
  • e9e0c56: 8314319: LogCompilation doesn't reset lateInlining when it encounters a failure.
  • 56b8db1: 8258970: Disabled JPasswordField foreground color is wrong with GTK LAF
  • 0d4cadb: 8315195: RISC-V: Update hwprobe query for new extensions
  • b4f7069: 8315446: G1: Remove unused G1AllocRegion::attempt_allocation
  • cf02cf3: 8315098: Improve URLEncodeDecode microbenchmark
  • c32e340: 8315321: [aix] os::attempt_reserve_memory_at must map at the requested address or fail
  • 42f5b9e: 8315436: HttpsServer does not send TLS alerts
  • 033f311: 8315069: Relativize extended_sp in interpreter frames
  • c2e01eb: 8313983: jmod create --target-platform should replace existing ModuleTarget attribute
  • ... and 36 more: https://git.openjdk.org/jdk/compare/ed1ea5fe7c6fad03ca96e7dece2127eab21a608a...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Sep 1, 2023
@turbanoff
Copy link
Member Author

Do we need a second reviewer or I can integrate?

@aivanov-jdk
Copy link
Member

Do we need a second reviewer or I can integrate?

It's a good question.

Overall, the changes are trivial: only comments are updated, some javadoc for private classes which are comments, then a couple of method names and variable names are corrected. I can't see any logic change.

I would say integrate if no else looks in the next 12–24 hours.

@aivanov-jdk
Copy link
Member

I reverted changes in non-Swing parts to reduce amount of files to review.

Did you submit a follow-up bug for those files?
Do you have a PR already?

@turbanoff
Copy link
Member Author

turbanoff commented Sep 13, 2023

Did you submit a follow-up bug for those files?
Do you have a PR already?

I plan to work on new PR after merge of this.

@turbanoff
Copy link
Member Author

/integrate

@openjdk
Copy link

openjdk bot commented Sep 15, 2023

Going to push as commit 89cb290.
Since your change was applied there have been 225 commits pushed to the master branch:

  • 25f32f3: 8316160: Remove sun.misc.Unsafe.{shouldBeInitialized,ensureClassInitialized}
  • 86dde5e: 8316001: GC: Make TestArrayAllocatorMallocLimit use createTestJvm
  • 4f864fa: 8314136: Test java/net/httpclient/CancelRequestTest.java failed: WARNING: tracker for HttpClientImpl(42) has outstanding operations
  • 4070829: 8315931: RISC-V: xxxMaxVectorTestsSmokeTest fails when using RVV
  • d575968: 8316178: Better diagnostic header for CodeBlobs
  • bfbc41c: 8315741: Open source few swing JFormattedTextField and JPopupMenu tests
  • 0775bf2: 8316106: Open source few swing JInternalFrame and JMenuBar tests
  • 4a63eb0: 8315834: Open source several Swing JSpinner related tests
  • 8dc2d92: 8316190: Improve MemorySegment::toString
  • 783e44d: 8316326: ProblemList java/awt/Mouse/EnterExitEvents/DragWindowTest.java on macosx-all again
  • ... and 215 more: https://git.openjdk.org/jdk/compare/ed1ea5fe7c6fad03ca96e7dece2127eab21a608a...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Sep 15, 2023
@openjdk openjdk bot closed this Sep 15, 2023
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 15, 2023
@openjdk
Copy link

openjdk bot commented Sep 15, 2023

@turbanoff Pushed as commit 89cb290.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client client-libs-dev@openjdk.org i18n i18n-dev@openjdk.org integrated Pull request has been integrated
2 participants