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

Add a GitHub action to publish The Book #877

Merged
merged 1 commit into from
Mar 5, 2020

Conversation

adaszko
Copy link
Contributor

@adaszko adaszko commented Feb 27, 2020

Issue Addressed

The Lighthouse Book is currently out of date.

Resolves #546

Proposed Changes

Add an GitHub action that builds and uploads (to S3) the latest version on every merge to master.

Additional Info

This PR has been shown to successfully build and upload (with hook branch set to update-book temporarily) http://adam-lighthouse-book-dev.s3-website-ap-southeast-2.amazonaws.com/: https://github.com/adaszko/lighthouse/runs/472525745?check_suite_focus=true
Hopefully, you're able to view the links above.

The plan for merging:

  1. Add 3 new secrets to the sigmap/lighthouse project:
  2. Merge this PR to master
  3. Observe if the first merged PR to master triggers a successful mdbook / build-and-upload-to-s3 job

@paulhauner
Copy link
Member

Awesome! I'll get to adding those secrets. Presently we're using the docs from the ethdenver branch, I'll sneak in a PR so I can get those to master before we merge in this one.

@paulhauner
Copy link
Member

Hey @adaszko, I'm happy to proceed after we merge in #892. Do you mind giving me a review, please?

@adaszko
Copy link
Contributor Author

adaszko commented Mar 4, 2020

Hey @adaszko, I'm happy to proceed after we merge in #892. Do you mind giving me a review, please?

Done!

- run: mdbook build
working-directory: book

- uses: jakejarvis/s3-sync-action@master
Copy link
Member

Choose a reason for hiding this comment

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

Are we able to pin this to a specific commit instead of master?

Just in case Jake decides to get nefarious :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You never now with Jake!

Copy link
Contributor Author

@adaszko adaszko Mar 5, 2020

Choose a reason for hiding this comment

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

Done ✅

@paulhauner paulhauner added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Mar 5, 2020
with:
args: --follow-symlinks --delete
env:
AWS_S3_BUCKET: ${{ secrets.AWS_S3_BUCKET }}
Copy link
Member

Choose a reason for hiding this comment

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

Oh perhaps we could set this to AWS_S3_BOOK_BUCKET in case we decide to make an action that publishes the cargo docs too?

Copy link
Member

Choose a reason for hiding this comment

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

You may not have seen it, but we also have this:

https://lighthouse-docs.sigmaprime.io/

It's probably well out of date by now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I renamed the bucket name secret.

As for https://lighthouse-docs.sigmaprime.io/, I think it's redundant. The go to place for code documentation is either crates.io or docs.rs. I think we should publish there by the Principle of Least Astonishment ;) Creating a GitHub v*.*.* tag seems like a proper trigger for that.

Copy link
Member

Choose a reason for hiding this comment

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

We unfortunately can't publish on crates.io because we have references to local crates (e.g., some_crate = { path = "../some_crate" }). We could go and publish everything as a separate crate but that becomes very painful when you're making inter-crate changes (e.g., need to republish or modify the Cargo.toml to test).

I always assumed that you can only publish on docs.rs if you publish on crates.io.. Perhaps this is a bad assumption.

Copy link
Member

Choose a reason for hiding this comment

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

Creating a GitHub v*.*.* tag seems like a proper trigger for that.

What's this tag? I'm not familiar with it.

Copy link
Contributor Author

@adaszko adaszko Mar 6, 2020

Choose a reason for hiding this comment

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

I always assumed that you can only publish on docs.rs if you publish on crates.io.. Perhaps this is a bad assumption.

That's a valid assumption despite how my wording might have indicated otherwise ;)

What's this tag? I'm not familiar with it.

Oh that's just a shell-inspired lingo for tags like v0.1.0, v0.1.1, etc. GitHub Actions support such wildcards: https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#filter-pattern-cheat-sheet

We unfortunately can't publish on crates.io because we have references to local crates (e.g., some_crate = { path = "../some_crate" }). We could go and publish everything as a separate crate but that becomes very painful when you're making inter-crate changes (e.g., need to republish or modify the Cargo.toml to test).

Didn't realize that. In that case, a link to https://lighthouse-docs.sigmaprime.io/ on the crates.io page should do. I've created an issue for publishing code docs: #897. Feel free to prioritize it in the 'Security Review' project.

Copy link
Member

Choose a reason for hiding this comment

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

Didn't realize that. In that case, a link to https://lighthouse-docs.sigmaprime.io/ on the crates.io page should do.

I might be misunderstanding, but we can't even have a crates.io page for Lighthouse. cargo publish will always fail due to those path-based crate links :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see now 👍

@paulhauner paulhauner added ready-to-squerge and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Mar 5, 2020
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

Thanks!

@paulhauner paulhauner merged commit a87e8c5 into sigp:master Mar 5, 2020
@paulhauner
Copy link
Member

paulhauner commented Mar 5, 2020

Looks like it worked here!

https://github.com/sigp/lighthouse/runs/489004938?check_suite_focus=true

I can't see the changes on the web yet, but perhaps it's a caching thing.

EDIT: it just updated, yay!

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.

Deploy book via CI
2 participants