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 publish-docs workflow #669

Merged
merged 17 commits into from
Dec 6, 2023
Merged

Fix publish-docs workflow #669

merged 17 commits into from
Dec 6, 2023

Conversation

jolierabideau
Copy link
Contributor

@jolierabideau jolierabideau commented Dec 5, 2023

Follow up to #666

  • Removed the matrix for node_version
  • The deploy action didn't seem to like that I was giving it two folders, so I added to the build documentation and put the two subdirectories into one docs-for-pages directory.
  • Add .nojekyll file so that HTML pages that start with _ will not return a 404
  • Create .github/assets that holds a landing page: github-pages-index.html. The deploy action overwrites the github-pages branch with each deploy, so this page needs to be housed somewhere off of the github-pages branch
  • Copy the landing page into docs-for-pages when deploying
  • Use the build script with --out folder name to skip renaming step
  • Remove npx from build scripts
  • Fix entryPoint for papi-components

Landing page:

Screenshot 2023-12-06 at 10 57 57 AM

/papi-components

Screenshot 2023-12-06 at 10 54 02 AM

/papi-dts

Screenshot 2023-12-06 at 10 54 46 AM


This change is Reviewable

Copy link
Contributor

@irahopkinson irahopkinson left a comment

Choose a reason for hiding this comment

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

These changes :lgtm:.

How do you know that main has access to push to github-pages? I see that you don't have the permissions section in publish-docs.yml that is indicated in https://github.com/JamesIves/github-pages-deploy-action#getting-started-airplane. An admin of our repo might also need to enable something for this to work at all.

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@irahopkinson irahopkinson left a comment

Choose a reason for hiding this comment

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

I just changed GH Pages Source in our repo to be GitHub Actions instead of Deploy from a branch. That might help.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

tjcouch-sil
tjcouch-sil previously approved these changes Dec 5, 2023
Copy link
Member

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

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

Were you still unable to deploy even after I removed the restrictions on the branch, or did that resolve the problem?

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jolierabideau)


.github/workflows/publish-docs.yml line 30 at r1 (raw file):

        run: | # need to specify --out so pages links at /{their name} instead of at /docs
          cd lib/papi-components
          npx typedoc --out papi-components

I believe you should be able to do something like the following to get command-line arguments into the script:

npm run build:docs -- --out papi-components

-- is a special escape argument you can provide to npm that basically says "put all the command-line arguments from here on into the script instead of being npm arguments

Copy link
Contributor Author

@jolierabideau jolierabideau left a comment

Choose a reason for hiding this comment

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

I wasn't sure that main had access to push there. I thought that could resolve the issue since main is protected and has more permissions than other branches would.

I just deployed by making the workflow run on push to this branch- fix-publish-docs. The links (https://paranext.github.io/paranext-core/papi-dts or https://paranext.github.io/paranext-core/papi-components) however are not working, so I may need to work with permissions like Ira suggested.

Reviewable status: 3 of 4 files reviewed, all discussions resolved (waiting on @irahopkinson and @tjcouch-sil)

tjcouch-sil
tjcouch-sil previously approved these changes Dec 5, 2023
Copy link
Member

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

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

Did the workflow finish?

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor Author

@jolierabideau jolierabideau left a comment

Choose a reason for hiding this comment

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

It did
https://github.com/paranext/paranext-core/actions/runs/7106649571/job/19346507871

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@irahopkinson irahopkinson left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Member

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

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

:lgtm: thanks for the hard work on this! So excited to have auto-generated docs.

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@jolierabideau jolierabideau merged commit 8e6028c into main Dec 6, 2023
7 checks passed
@jolierabideau jolierabideau deleted the fix-publish-docs branch December 6, 2023 17:35
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.

3 participants