Skip to content
This repository has been archived by the owner on Jun 28, 2022. It is now read-only.

Github CI lint and and tests #46

Merged
merged 2 commits into from
Apr 12, 2022
Merged

Github CI lint and and tests #46

merged 2 commits into from
Apr 12, 2022

Conversation

pawelchcki
Copy link
Contributor

@pawelchcki pawelchcki commented Apr 4, 2022

What does this PR do?

Adds basic CI Lints and Tests in Github actions.

  • Including a check for license headers in source files

Motivation

Quick feedback and lint check for submitted PRs

@pawelchcki pawelchcki marked this pull request as ready for review April 4, 2022 17:26
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

👍 Looks GREAT to me, thanks for this!

Comment on lines +7 to +16
test:
name: cargo test --workspace
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v2
- name: Cache
uses: ./.github/actions/cache
- run: cargo build --workspace --verbose
- run: cargo test --workspace --verbose
Copy link
Member

Choose a reason for hiding this comment

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

Can I convince you to also build the examples/ffi? We've had issues with them bitrotting in the past so it'd be great to have them running in CI :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we should definitely add that build to the CI. But it seems I need to basically build the end artifact to make cmake happy.

I have another proposal in progress, which builds the world using bazel its not handling the example yet.
But once it does - it would just be this invocation in the CI for build checking everything:

bazel build //...

Bazel or no bazel, we'll definitely need to check this in CI at some point. So I can take a note, to return to it, if we decide bazel sucks too much :)

Copy link
Contributor

@r1viollet r1viollet left a comment

Choose a reason for hiding this comment

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

This is nice !
Thanks for this PR

@pawelchcki pawelchcki merged commit e1bd12d into main Apr 12, 2022
@pawelchcki pawelchcki deleted the pull_github_actions branch April 12, 2022 08:17
@pawelchcki
Copy link
Contributor Author

Thanks all for the review !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants