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

WIP GitHub Actions for CI #83

Merged
merged 7 commits into from
May 13, 2020
Merged

Conversation

echeran
Copy link
Contributor

@echeran echeran commented May 12, 2020

@zbraniecki Here's my progress so far, in case you also play around on your personal fork.

@echeran echeran requested review from zbraniecki, srl295 and sffc May 12, 2020 20:38
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
.github/workflows/branch-build-test.yml Outdated Show resolved Hide resolved

## Cache steps

- name: Cache cargo registry
Copy link
Member

Choose a reason for hiding this comment

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

probably future but si there a way to refactor these caches into something importable with uses?

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'm not sure I understand. When you said uses, were you referring to a config key in the step of a workflow in the YAML file? That particular uses key just seems to hold the coordinates of the external action to invoke for that particular step within the workflow steps list.

The way that the Cache action works is interesting -- it hashes a file (for Rust, Cargo.lock) to know whether deps have changes or not.

I obviously just took the defaults for Rust - Cargo. But we can definitely try modifying if useful.

Copy link
Member

Choose a reason for hiding this comment

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

they are interesting… And I'm completely new to this, so just speculating.

Look at https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#example-using-action-in-the-same-repository-as-the-workflow - it suggests to me that we could move "Cache cargo registry" etc .to a separate file, and then just add: `uses: ./cache-cargo' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh, yeah, I think relative path syntax applies if we had a special ICU4X action that we created in our repo, and wanted to invoke it from within this workflow. The standard coordinates refer to a public repo on Github. The last option is referring to an action defined in a Docker image on Dock Hub.

As I imagine it, most workflows would just take advantage of pre-written actions, as need be.
Github own the "github.com/actions" project, so actions/checkout refers to their github.com/actions/checkout repo. As I understand it so far, actions/checkout clones your repo code into the runner environment, actions/cache fetches previously cached deps to start with or else saves the new deps at the end of the run.

The blog post tutorial @nciric shared uses another pre-built action that loads multiple specific versions of Golang, which was useful for version compatibility testing purposes.

Copy link
Member

Choose a reason for hiding this comment

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

@echeran this can be a future refinement. the caching you're doing seems like it could be useful separately.

README.md Outdated Show resolved Hide resolved
.github/workflows/branch-build-test.yml Outdated Show resolved Hide resolved
@echeran echeran requested review from sffc and srl295 May 13, 2020 00:34
Copy link
Member

@srl295 srl295 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@zbraniecki zbraniecki left a comment

Choose a reason for hiding this comment

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

LGTM!

@zbraniecki
Copy link
Member

That looks good! Let's land this and then I can try to plug code coverage into it later :) Thanks @echeran !

@echeran echeran merged commit 9c6b275 into unicode-org:master May 13, 2020
@echeran echeran linked an issue Jun 25, 2020 that may be closed by this pull request
@echeran echeran removed a link to an issue Jun 25, 2020
@echeran echeran linked an issue Jun 25, 2020 that may be closed by this pull request
@echeran echeran deleted the github-actions branch October 6, 2020 17:58
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.

Set up continous integration
4 participants