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

8251549: Update docs on building for Git #205

Closed

Conversation

@jerboaa
Copy link
Contributor

@jerboaa jerboaa commented Aug 6, 2021

Hi,

Now that OpenJDK 11u is fully on Git we should update the docs to no longer refer to Mercurial and use Git instead. The JDK 16 patch doesn't apply clean due to some differences in the file (e.g. referring to JDK 8 vs. JDK 11 in head). doc/building.html is the result of running make update-build-docs when the building.md file has the relevant changes.

Thoughts?


Progress

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

Issue

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk11u-dev pull/205/head:pull/205
$ git checkout pull/205

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 205

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk11u-dev/pull/205.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Aug 6, 2021

👋 Welcome back sgehwolf! 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.

Loading

@openjdk openjdk bot changed the title Backport 042734cc5b17302a8f2ecdf577511bd6d5ec5e22 8251549: Update docs on building for Git Aug 6, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Aug 6, 2021

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

Loading

@mlbridge
Copy link

@mlbridge mlbridge bot commented Aug 6, 2021

Webrevs

Loading

Copy link
Contributor

@shipilev shipilev left a comment

The *.md change looks good, but *.html has a lot of irrelevant changes. This is usually caused by a different version of pandoc used to generate the HTML file. Please run make update-build-docs on master and see what changes it brings. The usual way is to find the pandoc version that produces the minimal changes to HTML. If we don't do this, then HTML would change every time a future contributor changes MD again.

Loading

@jerboaa
Copy link
Contributor Author

@jerboaa jerboaa commented Aug 9, 2021

The *.md change looks good, but *.html has a lot of irrelevant changes. This is usually caused by a different version of pandoc used to generate the HTML file.

I see. Makes sense. Do you know which pandoc version we need? I have 2.9.2.1

Please run make update-build-docs on master and see what changes it brings. The usual way is to find the pandoc version that produces the minimal changes to HTML. If we don't do this, then HTML would change every time a future contributor changes MD again.

OK. I'll try to figure it out. Thanks for the sanity check!

Loading

@shipilev
Copy link
Contributor

@shipilev shipilev commented Aug 9, 2021

The *.md change looks good, but *.html has a lot of irrelevant changes. This is usually caused by a different version of pandoc used to generate the HTML file.

I see. Makes sense. Do you know which pandoc version we need? I have 2.9.2.1

Look at ./make/conf/jib-profiles.js in the 11u tree and in current master. Chances are there is no exact Pandoc version that produces the same HTML file exactly, but we probably want to have it as close to current version as possible. So that we generate the HTML file once here, and then stick with the particular Pandoc version going forward.

Loading

@jerboaa
Copy link
Contributor Author

@jerboaa jerboaa commented Aug 9, 2021

The *.md change looks good, but *.html has a lot of irrelevant changes. This is usually caused by a different version of pandoc used to generate the HTML file.

I see. Makes sense. Do you know which pandoc version we need? I have 2.9.2.1

Look at ./make/conf/jib-profiles.js in the 11u tree and in current master. Chances are there is no exact Pandoc version that produces the same HTML file exactly, but we probably want to have it as close to current version as possible. So that we generate the HTML file once here, and then stick with the particular Pandoc version going forward.

Thanks!

Found the needed version, I think. Seems to be 1.17.2
https://github.com/openjdk/jdk11u-dev/blob/master/make/devkit/createPandocBundle.sh#L34

Loading

@jerboaa jerboaa force-pushed the jdk-8251549-git-docs-backport branch from cce6074 to bbbd50e Aug 9, 2021
@jerboaa
Copy link
Contributor Author

@jerboaa jerboaa commented Aug 9, 2021

@shade Updated doc/building.html with with pandoc 1.17.2 now. Looks clean-ish to me. Thoughts?

Loading

Copy link
Contributor

@shipilev shipilev left a comment

Thanks! There are a few unrelated hunks still (formatting in cross-compilation tables), but I don't think we need to fret about those. Looks good.

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 9, 2021

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

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

8251549: Update docs on building for Git

Reviewed-by: shade, clanger

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

  • f4772b5: 8272602: [macos] not all KEY_PRESSED events sent when control modifier is used
  • 90d5293: 8272131: PhaseMacroExpand::generate_slow_arraycopy crash when clone null CallProjections.fallthrough_ioproj
  • 8f402b2: 8272628: Problemlist gc/stress/gcbasher/TestGCBasherWithCMS.java for x86_32
  • 0621d9d: 8226602: Test convenience reactive primitives from java.net.http with RS TCK
  • 6722cee: 8226319: Add forgotten test/jdk/java/net/httpclient/BodySubscribersTest.java
  • 5ce46d9: 8248403: AArch64: Remove uses of kernel integer types
  • 3db0637: 8272197: Update 11u GHA workflow with Shenandoah configurations
  • 55545f4: 8225082: Remove IdenTrust certificate that is expiring in September 2021
  • 12312a3: 8230841: Remove oopDesc::equals()
  • 8a470e1: 8209768: Refactor java/util/prefs/CheckUserPrefsStorage.sh to plain java test
  • ... and 35 more: https://git.openjdk.java.net/jdk11u-dev/compare/1538790fdcad884a40f9a91ddba8587affd034d6...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.

Loading

@openjdk openjdk bot added the ready label Aug 9, 2021
@jerboaa
Copy link
Contributor Author

@jerboaa jerboaa commented Aug 9, 2021

Thanks for the review!

Loading

Copy link
Contributor

@RealCLanger RealCLanger left a comment

LGTM, thanks for backporting.

Loading

@jerboaa
Copy link
Contributor Author

@jerboaa jerboaa commented Aug 23, 2021

/integrate

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 23, 2021

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

Your commit was automatically rebased without conflicts.

Loading

@openjdk openjdk bot closed this Aug 23, 2021
@openjdk openjdk bot added integrated and removed ready rfr labels Aug 23, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Aug 23, 2021

@jerboaa Pushed as commit ba097ca.

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

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants