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

Docs: Fix status reporting to throw on error #39663

Conversation

IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Feb 15, 2021

Doc deploy is currently broken, but hasn't been throwing. Dev docs may be quite out of date.

A post_status method was missing for the build bot config, meaning that on an error status no error was thrown and just the fall back method that returns nothing.

This is evident because the DOCUMENTER_KEY appears to be broken in the build system but was just being printed as an @error in a successful make deploy step? @fredrikekre / @staticfloat ?

This adds throws on "errored" or "failed" status so the build system can catch it.

cc. @mortenpi

doc/make.jl Outdated Show resolved Hide resolved
@mortenpi mortenpi added the docsystem The documentation building system label Feb 15, 2021
doc/make.jl Show resolved Hide resolved
Copy link
Contributor

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

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

I'd say this LGTM, but it might be good to hear if @fredrikekre sees any issues with this.

@fredrikekre
Copy link
Member

fredrikekre commented Feb 15, 2021

Why doesn't deploydocs error instead?

@IanButterworth
Copy link
Member Author

I believe it's because on GitHub actions there's more granularity than just success or fail, so the error is logged and post_status sets the api endpoints appropriately.

Making post_status do the throwing seems like the right way to do with how the dispatching is set up.

I defer to @mortenpi though.

@mortenpi
Copy link
Contributor

Doing the dispatch here allows us to fix the problem without having to make changes to Documenter.

But yea, arguably deploydocs should just throw too actually, since I think that part shouldn't fail unless there is a network or configuration problem.

@fredrikekre
Copy link
Member

JuliaDocs/Documenter.jl#1529 is better I think

@mortenpi
Copy link
Contributor

I would like to put JuliaDocs/Documenter.jl#1529 into a minor release though, which will probably be a while, so it might be worth merging this as is and removing it when we update Documenter to 0.27 here.

@IanButterworth IanButterworth mentioned this pull request Feb 19, 2021
52 tasks
@fredrikekre
Copy link
Member

Fixed in Documenter.

@IanButterworth IanButterworth deleted the ib/docbuild_status_handler branch March 18, 2021 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docsystem The documentation building system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants