-
Notifications
You must be signed in to change notification settings - Fork 520
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
Switch from Travis to GHA #1073
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
MDBOOK_VERSION=0.4.7 | ||
MDBOOK_LINKCHECK_VERSION=0.7.2 | ||
MDBOOK_TOC_VERSION=0.6.1 | ||
DEPLOY_DIR=book/html |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
name: CI | ||
|
||
on: | ||
push: | ||
branches: | ||
- master | ||
pull_request: | ||
schedule: | ||
# Run at 18:00 UTC every day | ||
- cron: '0 18 * * *' | ||
|
||
jobs: | ||
ci: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v2 | ||
with: | ||
# Set 0 to rebase later. | ||
fetch-depth: 0 | ||
|
||
- name: Read .env | ||
run: | | ||
. ./.env | ||
echo "MDBOOK_VERSION=${MDBOOK_VERSION}" >> $GITHUB_ENV | ||
echo "MDBOOK_LINKCHECK_VERSION=${MDBOOK_LINKCHECK_VERSION}" >> $GITHUB_ENV | ||
echo "MDBOOK_TOC_VERSION=${MDBOOK_TOC_VERSION}" >> $GITHUB_ENV | ||
echo "DEPLOY_DIR=${DEPLOY_DIR}" >> $GITHUB_ENV | ||
jyn514 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
- name: Cache binaries | ||
id: mdbook-cache | ||
uses: actions/cache@v2 | ||
with: | ||
path: | | ||
~/.cargo/bin | ||
key: ${{ runner.os }}-${{ env.MDBOOK_VERSION }}--${{ env.MDBOOK_LINKCHECK_VERSION }}--${{ env.MDBOOK_TOC_VERSION }} | ||
|
||
- name: Cache linkcheck | ||
uses: actions/cache@v2 | ||
with: | ||
path: | | ||
~/book/linkcheck | ||
key: ${{ runner.os }}-${{ hashFiles('./book/linkcheck') }} | ||
|
||
- name: Check line lengths | ||
if: github.event_name != 'schedule' | ||
run: | | ||
shopt -s globstar | ||
MAX_LINE_LENGTH=100 bash ci/check_line_lengths.sh src/**/*.md | ||
|
||
- name: Install latest nightly Rust toolchain | ||
if: steps.mdbook-cache.outputs.cache-hit != 'true' | ||
uses: actions-rs/toolchain@v1 | ||
with: | ||
toolchain: nightly | ||
override: true | ||
|
||
- name: Install Dependencies | ||
if: steps.mdbook-cache.outputs.cache-hit != 'true' | ||
run: | | ||
cargo install mdbook --version ${{ env.MDBOOK_VERSION }} | ||
cargo install mdbook-linkcheck --version ${{ env.MDBOOK_LINKCHECK_VERSION }} | ||
cargo install mdbook-toc --version ${{ env.MDBOOK_TOC_VERSION }} | ||
Comment on lines
+58
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't come up with a good way to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems fine to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess that means we'll have to manually update these tools more often? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth it to introduce some automation to update those dependencies? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, I guess we might be able to get dependabot setup, but I'm not sure if it's worth doing it here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIK dependabot only updates Cargo.toml or Cargo.lock so it doesn't help us. But an automation job or something else would be great as follow-up work.
Yes, we can still use |
||
|
||
- name: Check build | ||
if: github.event_name != 'schedule' | ||
run: | | ||
git switch -c ci | ||
git fetch origin master | ||
git rebase origin/master | ||
git log --oneline | head -n 10 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this rebase over master? Doesn't github do that automatically? What happens on merge conflicts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just copy-pasted from the travis script, this is no longer required now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (IIRC we introduced it to avoid broken link failures that are already fixed on master.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know that rust-lang/rust runs CI after rebasing against master; I don't think they do that explicitly and github handles it automatically. Not sure though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That sounds surprising to me, but a quick search for 'rebase' in rust's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC this was introduced when we did huge refactorings on librustc, which causes a ton of link failures. Now the broken rate is low, so we could remove rebasing, I think. |
||
mdbook build | ||
|
||
- name: Deploy to gh-pages | ||
if: github.event_name == 'schedule' && github.repository == 'rust-lang/rustc-dev-guide' | ||
run: | | ||
mdbook build | ||
touch "${{ env.DEPLOY_DIR }}/.nojekyll" | ||
cp CNAME "${{ env.DEPLOY_DIR }}" | ||
cd "${{ env.DEPLOY_DIR }}" | ||
rm -rf .git | ||
git init | ||
git config user.name "Deploy from CI" | ||
git config user.email "" | ||
git add . | ||
git commit -m "Deploy ${GITHUB_SHA} to gh-pages" | ||
git push --quiet -f "https://x-token:${{ secrets.GITHUB_TOKEN }}@github.com/${GITHUB_REPOSITORY}" HEAD:gh-pages |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the link-check cronjob?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I understand, all the steps in the file are executed when master or a PR is pushed to, and at a regular interval (cron job). Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're partially correct, but there are several instances of #1073 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's the same as what we have on travis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to not have this in a separate yml file inside of workflows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's worth having duplicate workflows with multiple files.