-
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 all commits
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,77 @@ | ||
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 | ||
env: | ||
MDBOOK_VERSION: 0.4.7 | ||
MDBOOK_LINKCHECK_VERSION: 0.7.2 | ||
MDBOOK_TOC_VERSION: 0.6.1 | ||
DEPLOY_DIR: book/html | ||
BASE_SHA: ${{ github.event.pull_request.base.sha }} | ||
steps: | ||
- uses: actions/checkout@v2 | ||
with: | ||
# linkcheck needs the base commit. | ||
fetch-depth: 0 | ||
|
||
- 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 != 'push' | ||
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 doesn't this run on push? 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'm not sure we should block the deployment by the line length failure. It will be caught by the cron job anyway. 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. Will the line-length check run for PRs? I was confused about whether 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.
For instance, I pushed this branch but the workflow wasn't triggered. |
||
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 | ||
run: mdbook build | ||
|
||
- name: Deploy to gh-pages | ||
if: github.event_name == 'push' && github.repository == 'rust-lang/rustc-dev-guide' | ||
LeSeulArtichaut marked this conversation as resolved.
Show resolved
Hide resolved
|
||
run: | | ||
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.