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

force version 1.0 of Documenter.jl #953

Merged

Conversation

ThomasBreuer
Copy link
Member

@ThomasBreuer ThomasBreuer commented Nov 2, 2023

(as in AbstractAlgebra.jl and Nemo.jl)

For that, I have changed the three cross-references to the JuliaInterface manual from relative
(pointing to ../assets/html/JuliaInterface/chap0_mj.html) to absolute
(pointing to https://oscar-system.github.io/GAP.jl/stable/assets/html/JuliaInterface/chap0_mj.html).

This is not really nice, but I did not find another solution. When Documenter.jl (in version 1.0 or higher) checks the document's structure, it does not accept relative links outside the build directory, and apparently there is no option to suppress this check.

(One could think of encoding the links in question as https://.. URLs in the document, and inserting a new step into the build process, after the checks of course, which just replaces these URLs by the intended relative ones. I think this would be absurd.
If it would be possible to insert inline HTML code then the solution would be easy, but apparently this is not intended.)

(as in AbstractAlgebra.jl and Nemo.jl)

For that, I have changed the three cross-references to
the JuliaInterface manual from relative
(pointing to `../assets/html/JuliaInterface/chap0_mj.html`)
to absolute
(pointing to `https://oscar-system.github.io/GAP.jl/stable/assets/html/JuliaInterface/chap0_mj.html`).

This is not really nice, but I did not find another solution.
When Documenter.jl (in version 1.0 or higher) checks the document's structure,
it does not accept relative links outside the build directory,
and apparently there is no option to suppress this check.

(One could think of encoding the links in question as `http://..` URLs
in the document, and inserting a new step into the build process,
after the checks of course, which just replaces these URLs by the
intended relative ones.  I think this would be absurd.)
@ThomasBreuer
Copy link
Member Author

ThomasBreuer commented Nov 2, 2023

I do not understand the test outputs:
Some tests pass, but when I look at the details then I find that "Build package" was the last step that was started, and this step got canceled or reported an error.
Some tests fail, and this happens also in one of the early steps, before the intended tests start.

And I do not understand how the changes from this pull request can cause this behaviour.

@fingolfin
Copy link
Member

Hmm, this situation is strange. After all, Documenter.jl explicitly has this "assets" feature to add files from outside, and so I don't understand why they would block it. Perhaps there is a "trick" we are missing?

Could you perhaps open an issue on the Documenter.jl repository, including the precise error you are getting, and with a request for guidance? I think "we need to bundle some pre-built HTML pages" is not that outlandish a request...

@fingolfin
Copy link
Member

As to the errors, they may be temporary? Let's close and re-open to see

@fingolfin fingolfin closed this Nov 3, 2023
@fingolfin fingolfin reopened this Nov 3, 2023
Copy link

codecov bot commented Nov 3, 2023

Codecov Report

Merging #953 (8d96ff8) into master (aee7200) will increase coverage by 0.04%.
Report is 1 commits behind head on master.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #953      +/-   ##
==========================================
+ Coverage   75.70%   75.74%   +0.04%     
==========================================
  Files          51       51              
  Lines        4169     4169              
==========================================
+ Hits         3156     3158       +2     
+ Misses       1013     1011       -2     

see 1 file with indirect coverage changes

@ThomasBreuer
Copy link
Member Author

@fingolfin Thanks for the feedback.
Yes, I will open an issue for Documenter.jl.

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

works now just fine

@fingolfin fingolfin merged commit bdf9923 into oscar-system:master Nov 7, 2023
14 checks passed
@ThomasBreuer ThomasBreuer deleted the TB_request_Documenter_v1 branch November 7, 2023 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants