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

Fix docsite publish #636

Merged
merged 6 commits into from
Dec 16, 2021
Merged

Conversation

DaniruKun
Copy link
Contributor

This PR fixes the docsite publishing, which was previously missing uploading and downloading of the docsite static files artefact.

@axelson
Copy link
Member

axelson commented Dec 6, 2021

@DaniruKun Thanks for this! Can you temporarily cause this to run on every PR? I want ensure the site gets built and I don't want to make another tag just to get the site built and deployed. Once it is built and deployed we can change it back to run only on tags and then merge this PR.

@DaniruKun DaniruKun force-pushed the fix-docsite-publish branch 3 times, most recently from 8f66b6c to 9b545dd Compare December 6, 2021 20:43
@DaniruKun
Copy link
Contributor Author

@axelson please make sure the mkdocs branch exists

@DaniruKun DaniruKun force-pushed the fix-docsite-publish branch from 9b545dd to 5c3f801 Compare December 6, 2021 20:53
@DaniruKun DaniruKun closed this Dec 6, 2021
@DaniruKun DaniruKun reopened this Dec 6, 2021
@DaniruKun DaniruKun force-pushed the fix-docsite-publish branch 5 times, most recently from 29bb44c to 99b228e Compare December 6, 2021 21:09
with:
branch: mkdocs
folder: site
token: ${{ secrets.GITHUB_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

Judging by the documentation the token shouldn't need to be passed directly, so we should be okay if we just remove this line.

Suggested change
token: ${{ secrets.GITHUB_TOKEN }}

@DaniruKun DaniruKun force-pushed the fix-docsite-publish branch from 99b228e to 3fb2ceb Compare December 7, 2021 18:30
This was referenced Dec 8, 2021
@axelson
Copy link
Member

axelson commented Dec 8, 2021

Okay, in #640 I've deployed the initial version of the site 🎉
https://elixir-lsp.github.io/elixir-ls/
I'm going to close 640 so now we can set this back to build on tags. Then this should be good to merge!

@DaniruKun
Copy link
Contributor Author

@axelson so no changes are needed in the workflow file, apart from trigger on new tag push on master?

@axelson
Copy link
Member

axelson commented Dec 11, 2021

@DaniruKun yes that's correct, but it should be built when there's a tag, not from pushes to master. The issue is that the build previously was being triggered from a PR, where the branch that backed the PR was from a fork, which meant that the token associated with the PR did not have the appropriate permissions. Now that we'll be triggering the workflow from a tag that shouldn't be an issue.

@axelson
Copy link
Member

axelson commented Dec 14, 2021

Okay, I changed it to build on tags in 00b7dde

Copy link
Member

@axelson axelson left a comment

Choose a reason for hiding this comment

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

Thank you! ❤️

@axelson axelson merged commit c6a4b23 into elixir-lsp:master Dec 16, 2021
@DaniruKun DaniruKun deleted the fix-docsite-publish branch December 16, 2021 18:38
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