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

chore: fix "docs ghpages/deploy" ci #305

Merged
merged 5 commits into from
Apr 4, 2024
Merged

Conversation

duguorong009
Copy link

@duguorong009 duguorong009 commented Apr 1, 2024

Description

  • fix the error in "docs ghpages/deploy" ci

Related issues

Changes

  • upgrade the toolchain to nightly-2023-10-05 in docs-ghpages.yml
  • move the katex-header.html file from ./halo2/ dir to .github/ dir
  • add the process of copying/deleting the html files needed for ci

Copy link
Member

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

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

Do we know if there's any way to have the JS scripts in a single place and then we import them to every crate? I think that would make it easier to maintain.

@@ -16,7 +16,7 @@ jobs:
- uses: actions/checkout@v3
- uses: actions-rs/toolchain@v1
with:
toolchain: nightly
toolchain: nightly-2023-10-05
Copy link
Member

Choose a reason for hiding this comment

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

Why do we fix to this specific version?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe in the opposite direction: why we need a dynamic, some times broken nightly versions? It could generate build problems without any clear gain.

I even changed to a fixed release version in zcash/halo2 to proper generate the book: zcash@7df93fd#diff-b8e1b9f11d0148dab38c89b9eaf5aa56792726c6cead6bf9e359f53faa0eedb3L15

Copy link
Author

@duguorong009 duguorong009 Apr 2, 2024

Choose a reason for hiding this comment

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

The reasons that I use the nightly-2023-10-05 include the followings:

  • options for cargo doc command needs nightly compiler
  • latest nightly compiler fails to compile the pathfinder_simd crate
  • nightly-2023-10-05 is the one zcash repo used(+ compile success)

@CPerezz
I think reason 2 and 3 gives you the answer

@adria0
FYI, the toolchain version you tried in the commit is for mdbook crate install.
zcash@7df93fd#diff-b8e1b9f11d0148dab38c89b9eaf5aa56792726c6cead6bf9e359f53faa0eedb3R28-R29
In addition, you can see that they use the nightly-2023-10-05 toolchain for cargo doc command(what we are discussing).
zcash@7df93fd#diff-b8e1b9f11d0148dab38c89b9eaf5aa56792726c6cead6bf9e359f53faa0eedb3R43-R54

]
});
});
</script>
Copy link
Member

Choose a reason for hiding this comment

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

Can you add an empty new line at the end of the file?

Copy link
Author

Choose a reason for hiding this comment

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

Done d58b4b8

Copy link

@davidnevadoc davidnevadoc left a comment

Choose a reason for hiding this comment

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

LGTM!

@duguorong009
Copy link
Author

Do we know if there's any way to have the JS scripts in a single place and then we import them to every crate? I think that would make it easier to maintain.

Done c1e823a

Copy link
Member

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

LGTM!

@duguorong009 duguorong009 merged commit b9a057d into main Apr 4, 2024
15 checks passed
@duguorong009 duguorong009 deleted the gr@fix-ci-docs-ghpages branch April 4, 2024 06:52
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.

5 participants