Documenter.jl icon indicating copy to clipboard operation
Documenter.jl copied to clipboard

Wrong 'success' status on GitHub Actions when DOCUMENTER_KEY is not defined

Open Lecrapouille opened this issue 3 years ago • 4 comments

Hi ! This may a noob remark, but I was doing my first steps on GitHub Actions with Documenter.jl I'm using a "classic" make.jl and at the beginning I did not generate DOCUMENTER_KEY but GitHub Actions said "success" (with green cross) while an error message from Documenter.jl tried to explain to me that keys were not generated. In addition, this critical error message was hidden by 'tons' of other minor log messages.

This could be nice to improve it when deploydocs fails to force GitHub Actions to fail and stop immediately and not let it logging other messages that will hide this critical error. This also would stop making beginners confusing : no commit made on the gh-pages on a successful action done.

PS: DocumenterTools.genkeys(FooBar) is very nice to give me GitHub links in where to store keys since, before calling this function, I was searching in GitHub where to store these keys: I was looking in my profile settings while expecting to be set inside the project settings.

Lecrapouille avatar May 24 '21 14:05 Lecrapouille

I don't think that it is a good idea to force Documenter to fail when the DOCUMENTER_KEY is unavailable. The reason is that private keys are unavailable to pull requests from people who don't have push access to a repository (for security reasons). This would mean that any CI job on a PR containing a Documenter job would fail.

From experience, I can tell you that it is very discouraging to see a PR that one has put much effort in to be flagged with a big red cross.

rikhuijzer avatar Jun 05 '21 14:06 rikhuijzer

I understand this could not be a red cross :) but this could not be a green tilde because documentation has not been generated (I mean in the case of the key owner). Usually, a grey badge "build: error" is set like this one https://api.travis-ci.org/Lecrapouille/OpenGlassBox.svg?branch=master&status=errored (in this case by timeout of the CI test) but you are right I missed the case of PR.

Lecrapouille avatar Jun 05 '21 15:06 Lecrapouille

Between #1529 and #1567, I believe we might have this sorted? Although it's currently only available on master.

mortenpi avatar Jun 06 '21 00:06 mortenpi

Hmm, okay, it looks like that's no the case. E.g. when deploying dev docs from master, if you do not have the keys, it will still pass. I agree that ideally we'd fail in that situation, but @rikhuijzer rightly points out that we do not want to start failing on e.g. PRs. So it would take some effort to figure out how to make deploydocs decide that reliably.

mortenpi avatar Aug 14 '21 02:08 mortenpi