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

Check for empty CI environment variables #1511

Merged
merged 5 commits into from
Feb 3, 2021

Conversation

knuesel
Copy link
Contributor

@knuesel knuesel commented Jan 27, 2021

This PR changes deployconfig.jl to check that DOCUMENTER_KEY, GITHUB_TOKEN and GITHUB_ACTOR are non-empty. Currently it only checks if they are defined.

Why the current behavior is a problem: Many (most?) Julia packages configure CI with the following in the docs job:

        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          DOCUMENTER_KEY: ${{ secrets.DOCUMENTER_KEY }}

(That's what's generated by PkgTemplates and suggested by the Documenter manual.)

This means that deploys don't work with the initial configuration: both variables are defined, but DOCUMENTER_KEY will be empty unless/until the user configures the secret. At deploy time, Documenter gives priority to DOCUMENTER_KEY so the deploy will fail.

With this PR, Documenter will fall back to GITHUB_TOKEN when DOCUMENTER_KEY is empty, so deploys work "out of the box" since the token is always defined.

The PR also includes small docs change:

  • Mention that push_review doesn't work from forks
  • Make it clearer that GITHUB_TOKEN and DOCUMENTER_KEY are not necessarily both needed

@knuesel
Copy link
Contributor Author

knuesel commented Jan 27, 2021

I just force-pushed a small change: I was using a bad test for empty string (!==, not reliable).

@mortenpi mortenpi added this to the 0.26.2 milestone Feb 2, 2021
Copy link
Member

@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.

Also LGTM. And thanks a lot for for the detailed rationale in the OP!

@mortenpi mortenpi merged commit 4b655e7 into JuliaDocs:master Feb 3, 2021
mortenpi pushed a commit that referenced this pull request Feb 9, 2021
* Mention that push_review doesn't work from forks
* Check for empty DOCUMENTER_KEY/GITHUB_TOKEN/GITHUB_ACTOR
* Clarify that deploy token and key are not both needed

(cherry picked from commit 4b655e7)
@mortenpi mortenpi mentioned this pull request Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants