Skip to content

JDK-8253936 Replace <code>...</code> with {@code ...} for java.sql #707

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

Closed
wants to merge 2 commits into from

Conversation

Vipin-Sharma
Copy link
Contributor

@Vipin-Sharma Vipin-Sharma commented Oct 16, 2020

... is replaced with {@code ...} in java.sql classes.
Please review and sponsor this change.


Progress

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

Testing

Linux x64 Windows x64 macOS x64
Build ✔️ (5/5 passed) ✔️ (2/2 passed) ✔️ (2/2 passed)
Test (tier1) ✔️ (9/9 passed) ✔️ (9/9 passed) ✔️ (9/9 passed)

Issue

Reviewers

Download

$ git fetch https://git.openjdk.java.net/jdk pull/707/head:pull/707
$ git checkout pull/707

@bridgekeeper
Copy link

bridgekeeper bot commented Oct 16, 2020

👋 Welcome back vsharma! 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 Oct 16, 2020
@openjdk
Copy link

openjdk bot commented Oct 16, 2020

@Vipin-Sharma 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.

@openjdk openjdk bot added the core-libs core-libs-dev@openjdk.org label Oct 16, 2020
@mlbridge
Copy link

mlbridge bot commented Oct 16, 2020

Webrevs

@mlbridge
Copy link

mlbridge bot commented Oct 16, 2020

Mailing list message from Lance Andersen on core-libs-dev:

Hi Vipin,

Have you generated the javadocs for this change? If not I will pull the changes over this weekend and take a look

Best
Lance
------------------

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
Lance.Andersen at oracle.com

@Vipin-Sharma
Copy link
Contributor Author

Hi @LanceAndersen,

I have generated javadoc before and after change. Following are 3 differences showing side by side comparison here.

DriverManager.java - line 417 (Change: CODE replaced with code)
d.getClass().getName() | d.getClass().getName()

ResultSet.java - line 3085 (Change: CODE replaced with code)
the result set concurrency is CONCUR_READ_ONLY | the result set concurrency is CONCUR_READ_ONLY

SQLClientInfoException.java - line 60 (Change: removed space before null)
null and the vendor code is initialized to 0. | null and the vendor code is initialized to 0.

All the above three changes doesn't change the effective view.

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.

Hi Vipin

Thank you for tackling this update. There are a few required changes below. I will also be going through the generated javadoc to see if anything jumps out as well.

Best
Lance

* <p>
* <strong>Note:</strong> There is no validation of {@code updateCounts} for
* overflow and because of this it is recommended that you use the constructor
* {@code BatchUpdateException(String reason, String SQLState,
* int vendorCode, long []updateCounts,Throwable cause) }.
* </p>
* @param reason a description of the exception
* @param updateCounts an array of <code>int</code>, with each element
*indicating the update count, <code>Statement.SUCCESS_NO_INFO</code> or
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a space before indicating

@@ -1,5 +1,5 @@
/*
* Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1996, 2013, 2020, Oracle and/or its affiliates. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

The copyright update is incorrect, it should be 1996, 2020

@@ -1,5 +1,5 @@
/*
* Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1996, 2013, 2020, Oracle and/or its affiliates. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please correct the copyright as mentioned above

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2013, 2020, Oracle and/or its affiliates. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix the copyright

@@ -1998,11 +1998,11 @@ void updateObject(int columnIndex, Object x, int scaleOrLength)
void updateLong(String columnLabel, long x) throws SQLException;

/**
* Updates the designated column with a <code>float </code> value.
* Updates the designated column with a {@code float } value.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the space after float

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2006, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2006, 2020, Oracle and/or its affiliates. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

please fix the copyright

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2001, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2001, 2020, Oracle and/or its affiliates. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

please fix the copyright

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2006, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2006, 2020, Oracle and/or its affiliates. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

please fix the copyright

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2001, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2001, 2020, Oracle and/or its affiliates. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

please fix the copyright

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2014, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2014, 2020, Oracle and/or its affiliates. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

please fix the copyright

@openjdk
Copy link

openjdk bot commented Nov 4, 2020

@Vipin-Sharma 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:

8253936: Replace <code>...</code> with {@code ...} for java.sql

Reviewed-by: lancea

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

  • 166c728: 8255858: Add debug agent support for storing thread names
  • a0ade22: 8255900: x86: Reduce impact when VerifyOops is disabled
  • 29db1dc: 8255886: Shenandoah: Resolve cset address truncation and register clash in interpreter LRB
  • 26e7ef7: 8252870: Finalize (remove "incubator" from) jpackage
  • 804bd72: 8255128: linux x86 build failure with libJNIPoint.c
  • 160759c: 8255838: Use 32-bit immediate movslq in macro assembler if 64-bit value fits in 32 bits on x86_64
  • 94ace03: 8255863: Clean up test/jdk/java/lang/invoke/defineHiddenClass/BasicTest.java
  • 2da9981: 8255862: Remove @SuppressWarnings from sun.misc.Unsafe
  • 5348298: 8255855: appcds/jigsaw/NewModuleFinderTest.java test failed due to unexpected NPE
  • c287170: 8254742: InputStream::readNBytes(int) result may contain zeros not in input
  • ... and 340 more: https://git.openjdk.java.net/jdk/compare/07ec35e2e50bde9d3cf9e35733837dfd377ef1ab...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) 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 Nov 4, 2020
@Vipin-Sharma
Copy link
Contributor Author

/integrate

@openjdk openjdk bot added the sponsor Pull request is ready to be sponsored label Nov 4, 2020
@openjdk
Copy link

openjdk bot commented Nov 4, 2020

@Vipin-Sharma
Your change (at version 0823d56) is now ready to be sponsored by a Committer.

@Vipin-Sharma
Copy link
Contributor Author

I think all steps are completed for this PR, please help sponsoring this.

@LanceAndersen
Copy link
Contributor

/sponsor

@openjdk openjdk bot closed this Nov 25, 2020
@openjdk openjdk bot added integrated Pull request has been integrated and removed sponsor Pull request is ready to be sponsored ready Pull request is ready to be integrated rfr Pull request is ready for review labels Nov 25, 2020
@openjdk
Copy link

openjdk bot commented Nov 25, 2020

@LanceAndersen @Vipin-Sharma Since your change was applied there have been 668 commits pushed to the master branch:

  • cfb175d: 8256508: Improve CompileCommand flag
  • 7aed9b6: 8256016: Dacapo24H.java failed with "assert(false) failed: unscheduable graph"
  • 26e6cb3: 8256489: Make gtest for long path names on Windows more resilient in the presence of virus scanners
  • 911f16d: 8257056: Submit workflow should apt-get update to avoid package installation errors
  • b0bd0c2: 8256755: Update build.tools.depend.Depend to handle record components in API signatures
  • 9aeadbb: 8256865: Foreign Memory Access and Linker API are missing NPE checks
  • 8cd2e0f: 8243315: ParallelScavengeHeap::initialize() passes GenAlignment as page size to os::trace_page_sizes instead of actual page size
  • cdb41ba: 8255904: Remove superfluous use of reflection in Class::isRecord
  • c45725e: 8256747: GitHub Actions: decouple the hotspot build-only jobs from Linux x64 testing
  • 79e57ac: 8256643: Terminally deprecate ThreadGroup stop, destroy, isDestroyed, setDaemon and isDaemon
  • ... and 658 more: https://git.openjdk.java.net/jdk/compare/07ec35e2e50bde9d3cf9e35733837dfd377ef1ab...master

Your commit was automatically rebased without conflicts.

Pushed as commit dee79d6.

💡 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
core-libs core-libs-dev@openjdk.org integrated Pull request has been integrated
Development

Successfully merging this pull request may close these issues.

2 participants