Skip to content

Add github actions workflows #253

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

Merged
merged 3 commits into from
Sep 30, 2019
Merged

Add github actions workflows #253

merged 3 commits into from
Sep 30, 2019

Conversation

k-nasa
Copy link
Member

@k-nasa k-nasa commented Sep 28, 2019

background

close: #246

@k-nasa k-nasa marked this pull request as ready for review September 28, 2019 12:57
@yoshuawuyts yoshuawuyts added the enhancement New feature or request label Sep 28, 2019
rustup default nightly
rustup component add rustfmt
cargo install mdbook
rustc --version
Copy link

Choose a reason for hiding this comment

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

Did you see the install-mdbook script? That way saves quite a bit of time compared to the cargo install mdbook.

However, the script is fragile because it ultimately relies on a specific release filename pattern and it would be nice if we could have a more general solution. (This is currently an issue for another CI related PR here).

I think a better way might be to create a docker image based on the official image (or slim or docs.rs) with all of the tools we need (deadlinks, mdbook, tarpaulin?) pre-installed. Then we could use the container.image setting to set the environment so that stuff is available without having to rebuild it.

Another github action could handle docker image related tasks.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree this would be really nice, and it would probably worth doing this in a follow-up!

Perhaps it's worth opening an issue for this on https://github.com/actions-rs/meta/issues also; I agree that having a general solution for this would probably be very valuable to a lot of people.

Copy link
Collaborator

Choose a reason for hiding this comment

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

cargo install mdbook should be avoided, it adds ~8 min of build time.

Copy link
Member Author

Choose a reason for hiding this comment

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

Wow, long. I will fix it tonight, so take a moment

Copy link
Contributor

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

This looks good to me; I think we should merge this and run it together with Travis for a little bit to see how it works out. If it turns out it works well we can migrate entirely by turning off Travis.

Thanks heaps!

@yoshuawuyts
Copy link
Contributor

bors r+

@skade
Copy link
Collaborator

skade commented Sep 29, 2019

Bors seems to hang.

k-nasa and others added 2 commits September 29, 2019 18:32
@k-nasa
Copy link
Member Author

k-nasa commented Sep 30, 2019

I fixed install mdbook. Please re-review 🙇

@skade
Copy link
Collaborator

skade commented Sep 30, 2019

bors r+

@skade
Copy link
Collaborator

skade commented Sep 30, 2019

Bors breaks on this. I'll merge it in good faith to test.

@skade skade merged commit 33d2191 into async-rs:master Sep 30, 2019
@k-nasa k-nasa deleted the add_github_ci branch October 1, 2019 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use GitHub Actions
3 participants