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

View Source on master docs are broken #9304

Closed
Blacksmoke16 opened this issue May 15, 2020 · 6 comments · Fixed by #9305
Closed

View Source on master docs are broken #9304

Blacksmoke16 opened this issue May 15, 2020 · 6 comments · Fixed by #9305

Comments

@Blacksmoke16
Copy link
Member

https://crystal-lang.org/api/master/Adler32.html

Clicking the link under Defined In, or any of the methods View source, will 404. It seems to be trying to navigate to the repo crystal-lang/crystal.git.

@straight-shoota
Copy link
Member

This seems to be because GitHub only recognized URLs without .git extension in the repo name. I have no idea where that comes from. Distribution scripts checks out https://github.com/crystal-lang/crystal without .git.

@straight-shoota
Copy link
Member

It seems one of the things that make this complicated is that master docs are not from a dedicated docs build with the dockerfile from distribution-scripts, but just a byproduct of the test_linux job.

This explains my confusion about the remote url ending with .git (#9304 (comment)).

I'd suggest to use the proper dist_docs job for that (test_all_platforms on master). This would give us some safety that release docs won't be generated differently from master docs.

WDYT @bcardiff ?

@bcardiff
Copy link
Member

No. Because master docs are compiled without docker images. I think that should remain the same. The updates of master docs happens on every commit, and not on nightly.

@straight-shoota
Copy link
Member

Why shouldn't master docs use the exact same process as release docs? This would mean we need to maintain duplicate doc generation workflows. And that's just going to fail. For example, currently master docs don't have the version selector because --json-config-url is not set. We'd need to add that to the test_linux job.
It just seems silly to not use the process we already have for that purpose.

@bcardiff
Copy link
Member

It is a different process. And I don't see how you want to unify them

  • on master docs are built with the non-release compiled compiler and uploaded directly on s3. There is no dependency on distribution-scripts for this step, nor it should be.
  • on tagged/maintenance release docs are built using a fresh release compiled compiler. Their publication is done manually. Their build process is in distributions-scripts so there is a single point to track how the whole release is done.
  • on nightly docs are built but not published because master will be always uploaded earlier. having a preview here might help but it does not removes the need for master

@straight-shoota
Copy link
Member

There is no dependency on distribution-scripts for this step, nor it should be.

Why should it not be? Building docs for crystal-lang.org/api/master should be considered as a distribution process because it publishes to the API docs.

And I don't see how you want to unify them

It could be pretty much the same as dist_docs jobs. There's no docker image with master compiler available, so this makes it a bit more complicated. But I think we can build it on latest. This would lead to errors when there are rare changes to the doc generator CLI, but overall it would be pretty fine. And it's certainly less error-prone than having two separate API doc workflows.

A better solution would probably be adding an optional step to the distribution-scripts Dockerfile which builds the compiler locally and can be used when generating master docs. That doesn't seem like it would be a big problem.

Alternatively, we could consider extracting the generalized API docs build process out of distribution-scripts to use it independently for master builds. This seems more complicated, though.

I'd simply prefer to have a single place for building API docs for crystal-lang.org/api. This issue and the missing version selector on master docs indicate that maintaining duplicate workflows is just confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants