Skip to content

8282657: Code cleanup: removing double semicolons at the end of lines #7268

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.

Sign up for GitHub

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

Jump to bottom
Closed
matteobaccan wants to merge 1 commit into from
Closed

8282657: Code cleanup: removing double semicolons at the end of lines #7268

matteobaccan wants to merge 1 commit into from

Conversation

@matteobaccan
Copy link
Contributor

@matteobaccan matteobaccan commented Jan 28, 2022 •
edited by openjdk bot
Loading

Hi

I have reviewed the code for removing double semicolons at the end of lines

all the best
matteo

Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8282657: Code cleanup: removing double semicolons at the end of lines

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7268/head:pull/7268
$ git checkout pull/7268

Update a local copy of the PR:
$ git checkout pull/7268
$ git pull https://git.openjdk.java.net/jdk pull/7268/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7268

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7268.diff

@bridgekeeper bridgekeeper bot added the oca Needs verification of OCA signatory status label Jan 28, 2022
@bridgekeeper
Copy link

bridgekeeper bot commented Jan 28, 2022

Hi @matteobaccan, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user matteobaccan" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

@matteobaccan
Copy link
Contributor Author

matteobaccan commented Jan 28, 2022

/signed

@openjdk
Copy link

openjdk bot commented Jan 28, 2022

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

  • build
  • client
  • compiler
  • core-libs
  • hotspot
  • i18n
  • javadoc
  • kulla
  • net
  • nio
  • security
  • serviceability

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 serviceability serviceability-dev@openjdk.org hotspot hotspot-dev@openjdk.org labels Jan 28, 2022
@bridgekeeper bridgekeeper bot added the oca-verify Needs verification of OCA signatory status label Jan 28, 2022
@bridgekeeper
Copy link

bridgekeeper bot commented Jan 28, 2022

Thank you! Please allow for up to two weeks to process your OCA, although it is usually done within one to two business days. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated!

@openjdk openjdk bot added kulla kulla-dev@openjdk.org i18n i18n-dev@openjdk.org javadoc javadoc-dev@openjdk.org security security-dev@openjdk.org nio nio-dev@openjdk.org build build-dev@openjdk.org client client-libs-dev@openjdk.org core-libs core-libs-dev@openjdk.org compiler compiler-dev@openjdk.org net net-dev@openjdk.org labels Jan 28, 2022
@bridgekeeper bridgekeeper bot removed oca Needs verification of OCA signatory status oca-verify Needs verification of OCA signatory status labels Feb 22, 2022
@bridgekeeper
Copy link

bridgekeeper bot commented Feb 25, 2022

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

@matteobaccan
Copy link
Contributor Author

matteobaccan commented Feb 25, 2022

Hi

I have pushed this PR about 1 month ago. Only 3 days ago OCA was accepted.
Now: what is the next step?

This is a cleanup PR that removes some double semicolons at the end of some lines inside the JDK code.

ciao
matteo

@TheShermanTanker
Copy link
Contributor

TheShermanTanker commented Mar 4, 2022 •
edited
Loading

Hi @matteobaccan

The next step would be to create a relevant issue on the tracker and set it to track this PR. Since you don't have the ability to create new issues on the JBS yet I'll help you create one, please rename your PR title to 8282657 and the system should take care of the rest and automatically mark your PR as ready for review after setting it to track the corresponding JBS entry.

Cheers,
Julian

@matteobaccan matteobaccan changed the title Code cleanup: removing double semicolons at the end of lines 8282657: Code cleanup: removing double semicolons at the end of lines Mar 4, 2022
@openjdk openjdk bot added the rfr Pull request is ready for review label Mar 4, 2022
@mlbridge
Copy link

mlbridge bot commented Mar 4, 2022

Webrevs

LanceAndersen
LanceAndersen approved these changes Mar 4, 2022
View reviewed changes
Copy link
Contributor

@LanceAndersen LanceAndersen left a comment

Choose a reason for hiding this comment

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

The changes look OK. The copyright year probably should be updated as part of this PR

@matteobaccan
Copy link
Contributor Author

matteobaccan commented Mar 4, 2022

Hi Lance

I can make a second commit updating the copyright year

Tell me if this is necessary

ciao
matteo

RogerRiggs
RogerRiggs suggested changes Mar 4, 2022
View reviewed changes
Copy link
Contributor

@RogerRiggs RogerRiggs left a comment

Choose a reason for hiding this comment

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

We usually request that these be be broken up by area to attract the appropriate reviewers and avoid eye-strain. The client modules are usually separated out, as are hotspot.
And corresponding tests.
This kind of change is pretty low value for the code base and requires the involvement of quite a few reviewers.
If you had ask ahead of time, I would have suggested finding something with more value.

irisclark
irisclark approved these changes Mar 4, 2022
View reviewed changes
Copy link
Member

@irisclark irisclark left a comment

Choose a reason for hiding this comment

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

Nice tidy of the code.

Is there anything that can be done to prevent re-introduction of this trivial problem? Perhaps a new Skara bot jcheck option similar to what is already in place for trailing whitespace?

bradfordwetmore
bradfordwetmore approved these changes Mar 4, 2022
View reviewed changes
Copy link
Contributor

@bradfordwetmore bradfordwetmore left a comment

Choose a reason for hiding this comment

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

LGTM also.

Similar suggestion for updating copyrights.

dholmes-ora
dholmes-ora approved these changes Mar 5, 2022
View reviewed changes
Copy link
Member

@dholmes-ora dholmes-ora left a comment

Choose a reason for hiding this comment

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

I eyeballed the diff file and all seems okay.

Thanks,
David

@TheShermanTanker
Copy link
Contributor

TheShermanTanker commented Mar 5, 2022

Nice, good work matteo

Should I change the JBS issue title to match the PR title, or is it preferred for the PR title to change?

@miraculix0815
Copy link

miraculix0815 commented Mar 5, 2022 •
edited by bridgekeeper bot
Loading

This PR changes a comment in javax/swing/RepaintManager.java. This commented out code should be removed altogether in another PR? Because its an System.out.println and because its commented out code.

@erikj79
Copy link
Member

erikj79 commented Mar 7, 2022

Should I change the JBS issue title to match the PR title, or is it preferred for the PR title to change?

They need to match. You can either do it manually, or change the title to just the bug number and the bot will change it for you.

@TheShermanTanker
Copy link
Contributor

TheShermanTanker commented Mar 7, 2022

Should I change the JBS issue title to match the PR title, or is it preferred for the PR title to change?

They need to match. You can either do it manually, or change the title to just the bug number and the bot will change it for you.

Alright, I can't change the title of the PR, I guess it'll be easier for me to change the issue instead

@openjdk
Copy link

openjdk bot commented Mar 7, 2022 •
edited
Loading

@matteobaccan 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:

8282657: Code cleanup: removing double semicolons at the end of lines

Reviewed-by: lancea, rriggs, ihse, prr, iris, wetmore, darcy, dholmes

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 393 new commits pushed to the master branch:

  • 1faa5c8: 8282686: Add constructors taking a cause to SocketException
  • 7194097: 8252577: HotSpot Style Guide should link to One-True-Brace-Style description
  • ef266d7: 8278296: Generalize long range check transformation
  • f0995ab: 8280404: Unexpected exception thrown when CEN file entry comment length is not valid
  • 8e70f4c: 8282224: Correct TIG::bang_stack_shadow_pages comments
  • e544e35: 8282621: G1: G1SegmentedArray remove unnecessary template parameter
  • 104e3cb: 8282696: Add constructors taking a cause to InvalidObjectException and InvalidClassException
  • 6fc73f7: 8282620: G1/Parallel: Constify is_in_young() predicates
  • 894ffb0: 8282713: Invalid copyright notice in new test added by JDK-8275715
  • 415bf44: 8275715: D3D pipeline processes multiple PaintEvent at initial drawing
  • ... and 383 more: https://git.openjdk.java.net/jdk/compare/cb8a82ee24881113af4eea04d7ce5963d18e9b83...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@LanceAndersen, @RogerRiggs, @magicus, @prrace, @irisclark, @bradfordwetmore, @jddarcy, @dholmes-ora) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Mar 7, 2022
@LanceAndersen
Copy link
Contributor

LanceAndersen commented Mar 7, 2022

What problem are you having editing the PR header? You should be able to do so as the author of the PR

@kevinrushforth
Copy link
Member

kevinrushforth commented Mar 7, 2022

What problem are you having editing the PR header? You should be able to do so as the author of the PR

Exactly. You should see an "Edit" button near the right edge of the PR title. See the attached image:

PR-title

@kevinrushforth
Copy link
Member

kevinrushforth commented Mar 7, 2022

But as the JBS title and PR title now match, this is a moot point.

@magicus
Copy link
Member

magicus commented Mar 7, 2022

@LanceAndersen @kevinrushforth TheShermanTanker is not the author of this PR, he's just assisting the author by creating the JBS issue.

@magicus
Copy link
Member

magicus commented Mar 7, 2022

/sponsor

@openjdk
Copy link

openjdk bot commented Mar 7, 2022

@magicus The change author (@matteobaccan) must issue an integrate command before the integration can be sponsored.

@kevinrushforth
Copy link
Member

kevinrushforth commented Mar 7, 2022

TheShermanTanker is not the author of this PR, he's just assisting the author by creating the JBS issue.

Ah, that explains it then.

@matteobaccan
Copy link
Contributor Author

matteobaccan commented Mar 7, 2022

/integrate

@openjdk openjdk bot added the sponsor Pull request is ready to be sponsored label Mar 7, 2022
@openjdk
Copy link

openjdk bot commented Mar 7, 2022

@matteobaccan
Your change (at version 1edc519) is now ready to be sponsored by a Committer.

@magicus
Copy link
Member

magicus commented Mar 7, 2022

/sponsor

@openjdk
Copy link

openjdk bot commented Mar 7, 2022

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

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Mar 7, 2022
@openjdk openjdk bot closed this Mar 7, 2022
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Mar 7, 2022
@openjdk
Copy link

openjdk bot commented Mar 7, 2022

@magicus @matteobaccan Pushed as commit ccad392.

💡 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

Reviewers

@irisclark irisclark irisclark approved these changes

+7 more reviewers

@RogerRiggs RogerRiggs RogerRiggs approved these changes

@magicus magicus magicus approved these changes

@LanceAndersen LanceAndersen LanceAndersen approved these changes

@prrace prrace prrace approved these changes

@jddarcy jddarcy jddarcy approved these changes

@bradfordwetmore bradfordwetmore bradfordwetmore approved these changes

@dholmes-ora dholmes-ora dholmes-ora approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

build build-dev@openjdk.org client client-libs-dev@openjdk.org compiler compiler-dev@openjdk.org core-libs core-libs-dev@openjdk.org hotspot hotspot-dev@openjdk.org i18n i18n-dev@openjdk.org integrated Pull request has been integrated javadoc javadoc-dev@openjdk.org kulla kulla-dev@openjdk.org net net-dev@openjdk.org nio nio-dev@openjdk.org security security-dev@openjdk.org serviceability serviceability-dev@openjdk.org

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.