.github/workflows/doc-build.yml: Fix generation of CHANGES.html by mkoeppe · Pull Request #35808 · sagemath/sage

mkoeppe · 2023-06-22T18:00:22Z

📚 Description

This new feature from #35652 was supposed to show the changed HTML files,
but a last minute change there destroyed it.

Here we fix it.

📝 Checklist

The title is concise, informative, and self-explanatory.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation accordingly.

⌛ Dependencies

…lt documentation around running doc-clean

kwankyu · 2023-06-23T09:12:14Z

.github/workflows/doc-build.yml

+          mv /sage/local/share/doc/sage/html/en/.git /sage/.git-doc
+          make doc-clean doc-uninstall sagelib-clean && git clean -fx src/sage
+          mkdir -p /sage/local/share/doc/sage/html/en/ && mv /sage/.git-doc /sage/local/share/doc/sage/html/en/
+          ./config.status && make doc-html


Move .git-doc to .git-doc? No .git?

Where is the code to generate CHANGES.html? No idea how this works.

Thanks, fixed now.

CHANGES.html is generated in the "Copy docs" step

Are you aware that L53 is very similar to L100 in the "Copy docs" step. This seems to be a mistake to me...

Yes, you are.

What is the net effect of L83 and L85. It is moving .git directory to and from .gitdoc. Why?

In particular, why mkdir -p /sage/local/share/doc/sage/html/en/. The directory already exists. Why create it again?

I'm moving the .git directory away and back to protect it from make doc-uninstall

… index files

kwankyu · 2023-06-24T21:18:54Z

Most of the files in changes in the documentation preview of this PR still seem spurious. There should be none?

mkoeppe · 2023-06-24T21:20:46Z

I think they are all caused by functions whose default arguments display a hex address

kwankyu · 2023-06-24T21:26:09Z

I don't know if wiping out sage version from the sidebar is a good idea...

If I would change it at all and if possible, then I would augment the version number with the PR number, like Sage 10.1.beta4#12345 from Sage 10.1.beta4

kwankyu · 2023-06-24T21:38:27Z

By second thought, I would not change the version number at all because checking if the version number appear correct in the documentation is itself a part of the purpose of the documentation preview.

kwankyu · 2023-06-24T21:42:13Z

I think they are all caused by functions whose default arguments display a hex address

Yeah. I see.

kwankyu · 2023-06-24T22:26:19Z

And how about providing diff separately for each changed html, instead of one giant diff HTML.diff?

If there is a free API service that show git diff nicely, then we may use that...

mkoeppe · 2023-06-24T22:39:21Z

I would not change the version number at all because checking if the version number appear correct in the documentation is itself a part of the purpose of the documentation preview.

Without making the version number equal between both versions, every single file would show up in the diff

kwankyu · 2023-06-25T04:15:14Z

Both should show the same latest beta version. No? But I guess there could be a mismatch, for example, for some short time before the latest beta appears in the docker image, or when the branch of a PR is not rebased onto the latest beta. Is your making-version-number-equal a provision for these situations?

mkoeppe · 2023-06-25T04:17:40Z

Yes, that's right, it's intended for these situations. I don't think they are uncommon.

kwankyu · 2023-06-25T04:26:17Z

I agree that they are common. But if the author recognizes the version mismatch, then he/she can just upload the branch merged with the latest beta, which I think is, anyway, a good thing to do. Also this is just optional.

On the other hand, we frequently create links to the doc built for a PR, then it is good that the doc is accompanied with the right beta version number.

I am not sure about this matter though. I would not insist for now.

mkoeppe · 2023-06-25T04:33:37Z

I'll change it so that the old copy gets updated with the version number of the PR branch.

…output

kwankyu · 2023-06-25T05:14:04Z

Thanks. It seems a good compromise.

kwankyu · 2023-06-25T05:25:53Z

This is of course a wishlist item.

And how about providing diff separately for each changed html, instead of one giant diff HTML.diff?

If there is a free API service that show git diff nicely, then we may use that...

There is https://diffy.org/. But it seems not work with a url.

github-actions · 2023-06-25T06:23:53Z

Documentation preview for this PR (built with commit ebde730; changes) is ready! 🎉

kwankyu

It works now.

mkoeppe · 2023-06-25T06:36:33Z

Thanks for the review!

tobiasdiez · 2023-06-27T00:39:09Z

This is of course a wishlist item.

And how about providing diff separately for each changed html, instead of one giant diff HTML.diff?
If there is a free API service that show git diff nicely, then we may use that...

There is https://diffy.org/. But it seems not work with a url.

It wouldn't be too hard to use https://diff2html.xyz/

.github/workflows/doc-build.yml: Save/restore the git repo of the bui…

9905d4e

…lt documentation around running doc-clean

kwankyu reviewed Jun 23, 2023

View reviewed changes

Matthias Koeppe added 2 commits June 23, 2023 08:44

Fixup

396229b

.github/workflows/doc-build.yml: For CHANGES.html, ignore git, image,…

cdd675d

… index files

.github/workflows/doc-build.yml: Only patch versions in the old HTML …

ebde730

…output

kwankyu approved these changes Jun 25, 2023

View reviewed changes

kwankyu added s: positive review p: critical / 2 c: documentation and removed s: needs review c: documentation labels Jun 25, 2023

vbraun merged commit 404d9e5 into sagemath:develop Jul 1, 2023

Uh oh!

Conversation

mkoeppe commented Jun 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📚 Description

📝 Checklist

⌛ Dependencies

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kwankyu commented Jun 24, 2023

Uh oh!

mkoeppe commented Jun 24, 2023

Uh oh!

kwankyu commented Jun 24, 2023

Uh oh!

kwankyu commented Jun 24, 2023

Uh oh!

kwankyu commented Jun 24, 2023

Uh oh!

kwankyu commented Jun 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkoeppe commented Jun 24, 2023

Uh oh!

kwankyu commented Jun 25, 2023

Uh oh!

mkoeppe commented Jun 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kwankyu commented Jun 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkoeppe commented Jun 25, 2023

Uh oh!

kwankyu commented Jun 25, 2023

Uh oh!

kwankyu commented Jun 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 25, 2023

Uh oh!

kwankyu left a comment

Choose a reason for hiding this comment

Uh oh!

mkoeppe commented Jun 25, 2023

Uh oh!

tobiasdiez commented Jun 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mkoeppe commented Jun 22, 2023 •

edited

Loading

kwankyu commented Jun 24, 2023 •

edited

Loading

mkoeppe commented Jun 25, 2023 •

edited

Loading

kwankyu commented Jun 25, 2023 •

edited

Loading

kwankyu commented Jun 25, 2023 •

edited

Loading