Skip to content
This repository was archived by the owner on Sep 20, 2022. It is now read-only.

.github/workflow : Added compile tests #79

Open
akshaim wants to merge 1 commit into
base: master
Choose a base branch
from
Open

.github/workflow : Added compile tests #79

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions .github/workflows/compile-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: Compilation tests for RIOT-OS submodule update

on:
push:
branches: [ master]
paths:
- RIOT
pull_request:
types: [opened, edited, reopened, synchronize]
branches: [ master ]
paths:
- RIOT

jobs:
build:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
applications: [coap-chat, sniffer, spectrum-scanner]

steps:
- uses: actions/checkout@v2
with:
submodules: true

- name: Pull the latest riotbuild docker image
run: docker pull riot/riotbuild:latest

- name: Run compile tests
run: |
make BUILD_IN_DOCKER=1 -C ${{ matrix.applications }}
Copy link

@MrKevinWeiss MrKevinWeiss Sep 12, 2022

Choose a reason for hiding this comment

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

I tried the following locally, this would all apps to be added without having to touch the CI list in order for testing.

Suggested change
make BUILD_IN_DOCKER=1 -C ${{ matrix.applications }}
export RIOTAPPS=$(find . -name '*Makefile*' -printf "%h\n" | sort -u | grep -v RIOT)
for appdir in $RIOTAPPS; do make BUILD_IN_DOCKER=1 -C $appdir; done

Copy link
Member

@miri64 miri64 Sep 19, 2022

Choose a reason for hiding this comment

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

I tried the following locally, this would all apps to be added without having to touch the CI list in order for testing.

This would however disable parallel testing. I personally prefer to use a CI matrix if there is one.

Copy link

@MrKevinWeiss MrKevinWeiss Sep 19, 2022

Choose a reason for hiding this comment

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

That means every test we would have to add into a list, also touching the github config, which also means that any non-maintainer would have to wait for it to be triggered.

Either way, AFAIK, we are merging this repo with the examples in the main RIOT repo. This would reduce all the duplicate CI work I suppose...

Copy link
Member

@miri64 miri64 Sep 19, 2022

Choose a reason for hiding this comment

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

Only 10% of changes in the past were file additions and not all of those were new applications (given that this repo only contains 3 applications at the moment)

$ git log --oneline --diff-filter=A | wc -l
14
$ git log --oneline | wc -l
140

I think we would manage ;-). But yes, given RIOT-OS/RIOT#18602 this might indeed be a mood point.