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

8273497: Building.md should link to testing md file rather than html #5417

Closed
wants to merge 1 commit into from

Conversation

DanHeidinga
Copy link
Contributor

@DanHeidinga DanHeidinga commented Sep 8, 2021

Discovered while working through the building guide.


Progress

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

Issue

  • JDK-8273497: Building.md should link to testing md file rather than html

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5417

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

Using diff file

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

@bridgekeeper
Copy link

bridgekeeper bot commented Sep 8, 2021

👋 Welcome back DanHeidinga! 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 Sep 8, 2021
@DanHeidinga DanHeidinga changed the title 8273497: Change from testing.html to testing.md 8273497: Building.md should link to testing md file rather than html Sep 8, 2021
@openjdk
Copy link

openjdk bot commented Sep 8, 2021

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

  • build

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 build build-dev@openjdk.org label Sep 8, 2021
@mlbridge
Copy link

mlbridge bot commented Sep 8, 2021

Webrevs

@erikj79
Copy link
Member

erikj79 commented Sep 8, 2021

When updating the .md doc files, you must also run the make target to generate the corresponding .html files and submit them together. In this case, changing the link to .md will change both the building.md and building.html file. I'm guessing that the original author assumed that the only file that would be browsed online was the html file, so it made sense to assume links to only apply to html files. This was back when we hosted the source on Mercurial. These days, online browsing of .md files is standard practice on sites like Github, so we may need to rethink this.

I don't think just changing the link here is the correct action. I see these potential options:

  1. Create two links and mark them with .md and .html so that the reader may pick the one that makes sense depending on context.
  2. Get rid of the html files since Github now naturally uses .md files rather than .html files for online reading.
  3. Do nothing and continue to consider the html files as the official online documentation files.

I'm thinking 2 may be the preferred action, but would like more input from others. It would certainly make maintaining these files simpler as we no longer require the correct pandoc version present to update them.

@shipilev
Copy link
Member

shipilev commented Sep 8, 2021

I don't think just changing the link here is the correct action.

+1. Let's not do this change yet.

1. Create two links and mark them with .md and .html so that the reader may pick the one that makes sense depending on context.
2. Get rid of the html files since Github now naturally uses .md files rather than .html files for online reading.
3. Do nothing and continue to consider the html files as the official online documentation files.

I'm thinking 2 may be the preferred action, but would like more input from others.

(2) looks appealing for me, for a different reason: if you regenerate .md -> .html, and choose the unexpected pandoc version (for example one provided by distro), then .html diff would have a lot of fluff not related to the actual change. And that would keep happening as people regenerate .html with their own versions of pandocs :) Removing .html from repo resolves this problem at its core.

We can still rewire make update-build-docs to e.g. make generate-build-docs and put the resulting HTML to build/ somewhere, so whoever deploying the HTML files on their site can still get it easily.

@DanHeidinga
Copy link
Contributor Author

(2) looks appealing for me, for a different reason: if you regenerate .md -> .html, and choose the unexpected pandoc version (for example one provided by distro), then .html diff would have a lot of fluff not related to the actual change. And that would keep happening as people regenerate .html with their own versions of pandocs :) Removing .html from repo resolves this problem at its core.

It still leaves the original question though - should the .md file link to the .html or the .md version? My preference is the .md file as markdown is more readable, IMO, for both offline and github viewing.

We can still rewire make update-build-docs to e.g. make generate-build-docs and put the resulting HTML to build/ somewhere, so whoever deploying the HTML files on their site can still get it easily.

I looked for links to the HTML files and found some on the build group's page [1] and in the "How to contribute" page [2]. Overall usage is inconsistent as the markdown files are also linked from related pages [3].

Are both versions actually needed?

[1] http://openjdk.java.net/groups/build/
[2] http://openjdk.java.net/contribute/
[3] http://openjdk.java.net/guide/#building-the-jdk

@mlbridge
Copy link

mlbridge bot commented Sep 8, 2021

Mailing list message from erik.joelsson at oracle.com on build-dev:

On 2021-09-08 10:44, Dan Heidinga wrote:

On Wed, 8 Sep 2021 16:14:36 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

(2) looks appealing for me, for a different reason: if you regenerate `.md` -> `.html`, and choose the unexpected pandoc version (for example one provided by distro), then `.html` diff would have a lot of fluff not related to the actual change. And that would keep happening as people regenerate `.html` with their own versions of pandocs :) Removing `.html` from repo resolves this problem at its core.

It does indeed. We have a long history have keeping generated files in
the repository, and getting rid of them is a good thing on its own.

It still leaves the original question though - should the `.md` file link to the `.html` or the `.md` version? My preference is the `.md` file as markdown is more readable, IMO, for both offline and github viewing.

It was intended as implicit in option 2 that the links should then all
be to pointing to .md since we are removing the .html files.

We can still rewire `make update-build-docs` to e.g. `make generate-build-docs` and put the resulting HTML to `build/` somewhere, so whoever deploying the HTML files on their site can still get it easily.
I looked for links to the HTML files and found some on the build group's page [1] and in the "How to contribute" page [2]. Overall usage is inconsistent as the markdown files are also linked from related pages [3].

I don't know of anyone uploading these .html files anywhere. As I
understand it, they were added in the current form because on our old
Mercurial servers, they could be conveniently browsed in raw form
directly from the repository, much like .md files are easily browsed on
Github. So my best guess is that they aren't needed, hence suggesting
option 2.

This is basically something we haven't quite cleaned up since moving to
Github.

/Erik

@shipilev
Copy link
Member

shipilev commented Sep 9, 2021

(2) looks appealing for me, for a different reason: if you regenerate .md -> .html, and choose the unexpected pandoc version (for example one provided by distro), then .html diff would have a lot of fluff not related to the actual change. And that would keep happening as people regenerate .html with their own versions of pandocs :) Removing .html from repo resolves this problem at its core.

It still leaves the original question though - should the .md file link to the .html or the .md version? My preference is the .md file as markdown is more readable, IMO, for both offline and github viewing.

Yes, it does leave the original question unanswered, apologies.

I would suggest to change all .html -> .md links in the PR that removes .html completely. It would be a larger PR and would probably require some buy-ins, but I think those are relatively simple. Removing HTMLs would not introduce any ambiguity with these link rewrites.

At the same time, I have no problems with this tiny PR as well.

@mlbridge
Copy link

mlbridge bot commented Sep 9, 2021

Mailing list message from Magnus Ihse Bursie on build-dev:

Ok, let me shed some light on this discussion. :-)

First, as Erik assumed, the link was created only for the html version,
since this was the only way in which clickable links made sense. If you
read the markdown file at the terminal, you'd still had to type "less
testing.md" or whatever, so no formal link would make sense.

This premise has changed with the advent of GitHub, and more advanced
markdown editors.

Secondly, the generated html file is by no means dead! It is still the
official build description, as published at
https://openjdk.java.net/groups/build/doc/building.html. (This URL
"redirects" to the latest version of building.html on github.)

I made a lot of effort some years ago to purge all sites from different
kinds of links to different, bad and/or outdated build instructions, and
replace them with this new canonical URL. I am sad to see that I have
still not succeeded, and that even the new official Guide provides an
incorrect link. I will submit a fix for this. The "How to contribute"
page is *really* old and broken. I think the plan is to remove it
completely and replace it with the Guide, when it get's to a more mature
state.

The problem here is that the markdown parser used by Github is quite
simplistic, and do not allow for the OpenJDK branded css formatting, nor
some of the markdown syntax that is used in building.md and testing.md.
So clicking on the building.md on Github gives you a rough idea what it
is about, but it is not presented as intended.

I do agree though that we should add a link to the markdown version as
well, so it is easy to click through to that. Basically, option 1 as
Erik listed them.

Since changes in building.md requires running pandoc using a make
target, I can offer to take over the bug and do the fix for you, if you
want.

/Magnus

On 2021-09-08 19:44, Dan Heidinga wrote:

@DanHeidinga
Copy link
Contributor Author

I do agree though that we should add a link to the markdown version as
well, so it is easy to click through to that. Basically, option 1 as
Erik listed them.

That works for me. It meets my basic requirement to get from the building.md -> testing.md without having to manually modify the url.

Since changes in building.md requires running pandoc using a make
target, I can offer to take over the bug and do the fix for you, if you
want.

That would be appreciated. It saves me from figuring out the right pandoc setup.

It does make me wonder though - is the overhead of running pandoc on markdown files for trivial changes like this worth it to provide OpenJDK branded css formatting? Would it be better to just unconditionally link to the markdown for this purpose? I don't know the history around this setup so asking as a new person to the project.

@mlbridge
Copy link

mlbridge bot commented Sep 9, 2021

Mailing list message from Magnus Ihse Bursie on build-dev:

On 2021-09-09 14:34, Dan Heidinga wrote:

On Thu, 9 Sep 2021 10:35:32 GMT, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:

I do agree though that we should add a link to the markdown version as
well, so it is easy to click through to that. Basically, option 1 as
Erik listed them.
That works for me. It meets my basic requirement to get from the building.md -> testing.md without having to manually modify the url.

Since changes in building.md requires running pandoc using a make
target, I can offer to take over the bug and do the fix for you, if you
want.
That would be appreciated. It saves me from figuring out the right pandoc setup.

Ok, I'll do that.

It does make me wonder though - is the overhead of running pandoc on markdown files for trivial changes like this worth it to provide OpenJDK branded css formatting? Would it be better to just unconditionally link to the markdown for this purpose? I don't know the history around this setup so asking as a new person to the project.

What you're basically saying is "I just want to view the markdown on
Github, and that's good enough for me". And that might indeed be good
enough for anybody. The move to Github is still relatively new in the
glacial tempo of the JDK, so we have not really reconsidered how or if
this changes things such as this. But to be clear: it is not just about
the css branding, it is also other markdown formatting that Github does
not know about (markdown is more of a family of loosely similar
standards, than a carefully specified format). If we want to go that
route, we need to reformat the documents to match what Github knows
about (and then it might conflict with pandoc...).

In addition to this, we have in general been very strict about not tying
official parts of the project to a specific VCS hosting company, so we
can easily move away from Github if the need should be. Converting the
markdown to confirm with Github standards is a move in the contrary
direction. This might be acceptable, but at the very least needs some
consideration.

/Magnus

@DanHeidinga
Copy link
Contributor Author

In addition to this, we have in general been very strict about not tying
official parts of the project to a specific VCS hosting company, so we
can easily move away from Github if the need should be. Converting the
markdown to confirm with Github standards is a move in the contrary
direction. This might be acceptable, but at the very least needs some
consideration.

Thanks Magnus. The background on the current situation / approach helps a lot.

@DanHeidinga
Copy link
Contributor Author

Replaced by #5451

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build build-dev@openjdk.org rfr Pull request is ready for review
3 participants