8360122: Fix java.sql\Connection.java indentation

[gustavosimon]

8360122: Refine formatting of Connection.java interface

---

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-8360122: Fix java.sql\Connection.java indentation (Bug - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25925

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

Using diff file

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

Using Webrev

Link to Webrev Comment

---

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-8360122: Fix java.sql\Connection.java indentation (Bug - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25925

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back gustavosimon! 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]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@gustavosimon The following label will be automatically applied to this pull request:

• core-libs

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

[mlbridge]

Webrevs

• 03: Full - Incremental (21696cf7)
• 02: Full - Incremental (6d43550b)
• 01: Full - Incremental (3a7d0b5a)
• 00: Full (4310eaf9)

[gustavosimon]

@liach Can you review this PR or assign someone that might be able to review?

[RogerRiggs]

The indentation fixes look ok.
The reformatting of declarations seem unnecessary and create lines that are longer than desirable to be able to do side-by-side compares (100 char max).
Generally, avoid just asking the IDE to re-format the source, it leads to unnecessary changes.

[liach]

FYI people don't usually review on weekends (you opened this PR in a weekend) or holidays. This PR is sent to core-libs-dev list so people will eventually see and review it.

[gustavosimon]

The indentation fixes look ok. The reformatting of declarations seem unnecessary and create lines that are longer than desirable to be able to do side-by-side compares (100 char max). Generally, avoid just asking the IDE to re-format the source, it leads to unnecessary changes.

I'm going to undo the formatting changes where lines exceed 100 characters. Actually, I didn’t ask the IDE to reformat; I only suggested this formatting to improve readability. However, it can make comparisons in the diff view more difficult.

@RogerRiggs Thanks for the review! 😃

[gustavosimon]

FYI people don't usually review on weekends (you opened this PR in a weekend) or holidays. This PR is sent to core-libs-dev list so people will eventually see and review it.

Great, @liach! Thanks for the information! 😄

[gustavosimon]

@RogerRiggs I have just commited the changes to not exceed the 100 chars limit.

[RogerRiggs]

I haven't quite identified your preferred formatting style.
From previous reformattings with multiple arguments, the exception was placed on a line by itself.

(Generally, we defer to the original/previous author's choice of line breaks and formatting.
Choosing only to reformat to use a consistent style within each file/class or package.)

The issue title and PR title should be a bit more general, its more than just indentation cleanup.

[gustavosimon]

@RogerRiggs My preferred formatting style is like this:

    default boolean setShardingKeyIfValid(ShardingKey shardingKey,
                                          ShardingKey superShardingKey,
                                          int timeout) throws SQLException {

But as you said, there is previous reformattings that does not follow it. If you agree, I will fix them. What do you thing about it?

About the issue title and PR title, we can make it more general like:

"Improve Readability and Maintainability of Connection.java"

What your suggestion on it?

[RogerRiggs]

I would backout the changes to signatures, fixing the bad indentation and leave it at that.
Lance is one of the long time maintainers of the SQL classes and will want to take a look.

[gustavosimon]

Great! I will do that.

[LanceAndersen]

I might not have time this week to look at this as I have some other commitments. I do think we will want to narrow the changes being proposed

[liach]

The current version looks much better.