Adding mdbook support/CI for PRs

[simonsan]

Fixes #6

Output: https://simonsan.github.io/std-dev-guide/

@KodrAus Feedback welcome :)

[camelid]

Is this based on equivalent code for the rustc-dev-guide? Ideally we'd keep the infra code in sync somehow...

[KodrAus]

I think the rustc dev guide is using Travis CI for its mdbook support. Was there a reason not to use Actions on that side too?

[camelid]

Was there a reason not to use Actions on that side too?

Not sure, but I think we're going to try to switch to Actions. And new stuff is being put on Actions: rust-lang/rustc-dev-guide#1037

[JohnTitor]

IIRC third-party actions shouldn't be used generally for security reasons (I think @pietroalbini has some thoughts about it).

I think the rustc dev guide is using Travis CI for its mdbook support. Was there a reason not to use Actions on that side too?

It's just not updated yet, we have rust-lang/rustc-dev-guide#940 but it isn't done.

[KodrAus]

IIRC third-party actions shouldn't be used generally for security reasons

Is this just for third-party actions that we pass secrets into? Or can any third-party action access secrets without us explicitly passing them through? I'd be interested to hear more about that.

[JohnTitor]

Given rust-lang/crates.io#2951 (comment) I think we should avoid using them regardless of secrets.

[KodrAus]

Thanks @JohnTitor, that seems fair to me.

@simonsan Maybe we could look at what it would take to inline the pieces of these actions we actually use? If we want to move the Rustc guide to Actions too at some point it seems like we may as well try get something working with it here too, then look at making things consistent.

[camelid]

For the "setup mdBook" part I would think you could just copy the commands that are run in Travis from the rustc-dev-guide since they're just shell commands. Not sure about the deploy; you might want to look through the .travis.yml file and the ci/ directory in rust-lang/rustc-dev-guide.

[KodrAus]

I imagine the deploy would be the usual dance of creating an empty Git repository, copying the HTML content into it and force pushing that to the gh-pages branch.

[camelid]

I think the rustc-dev-guide actually just force-pushes to a branch called gh-pages.

I think this is the responsible CI code:

https://github.com/rust-lang/rustc-dev-guide/blob/85c995de1b29f942a85b10dd5175412cc5d32c42/.travis.yml#L27-L34

I don't know Travis though, so I can't explain it :)

[simonsan]

These scripts in ./ci there is a markdown lint action, we use in patterns:

  markdown-lint:
    runs-on: ubuntu-20.04
    steps:
      - uses: actions/checkout@v2
      - name: Lint all files recursively
        uses: avto-dev/markdown-lint@v1
        with:
          config: '.markdownlint.yaml' 
          args: '**/*.md'

Would you also not consider using this and instead use the script? MDL is easy to setup with a yaml file:
https://github.com/rust-unofficial/patterns/blob/master/.markdownlint.yaml

Same for markdown link check:

  markdown-link-check:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@master
    - uses: gaurav-nelson/github-action-markdown-link-check@v1
      with:
        use-verbose-mode: 'yes'
        use-quiet-mode: 'yes'
        config-file: '.github/workflows/url-check-config.json'
        check-modified-files-only: 'yes'

These actions shouldn't modify anything and push something to branches and mostly run during a PR, so I'm not sure whether there should be any security concerns about it, while I understand these for mdbook actions and deployment.

[KodrAus]

Thanks for working on this @simonsan! Once we update the variables this looks good to me.

[simonsan]

@KodrAus Should I remove the .travis.yml?

[simonsan]

@KodrAus I've added mdbook build/test to test the building process of the book and the code samples during PR. Do you think that makes sense? I have prepared fixes for the files already so mdbook test runs through successfully that I can PR as well.

[simonsan]

Blocking:
#10 - One link is broken and one link is missing
#11 - Makes code sample tests pass for now

[KodrAus]

Thanks @simonsan! I merged in your fixes.

Removing the .travis.yml sounds good to me too 🙂 Save Travis a little compute.

[simonsan]

Removing the .travis.yml sounds good to me too 🙂 Save Travis a little compute.

Done :)

[KodrAus]

Thanks so much for working on this @simonsan! 🙇

[simonsan]

@KodrAus Seems there is still an issue with that CNAME. Anything to fix from my side?

[KodrAus]

@simonsan It's all good, we've got #12 opened to track getting that sorted. Thanks again for working on this!