Skip to content
Merged
Changes from all commits
Commits
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
83 changes: 83 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
name: PR Check CI

on:
pull_request:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we not also run on the push event? Also I think this restricts to pull requests off master which might not be what we want if there is a long running feature branch. I think is can just be this.
on: [push, pull_request]

Copy link
Contributor

Choose a reason for hiding this comment

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

I see that it by default includes synchronize so probably fine to leave it, but I would drop the branch. Maybe we still want to keep the push but limited to branch master so there is always a build on master
something like this:

on:
  pull_request
  push:
    branches: [master]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@liuguo09 also if you have your Dockerfile somewhere I would be happy to submit a PR against it to improve it a bit. There is some low hanging fruit that would reduce the size by quite a bit.

Yes, that's great. I meant to add the Dockerfile into nuttx testing repo. I'll add it later. Then you could update it there.

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 see that it by default includes synchronize so probably fine to leave it, but I would drop the branch. Maybe we still want to keep the push but limited to branch master so there is always a build on master
something like this:

on:
  pull_request
  push:
    branches: [master]

Yes, besides master branch, pull request check for feature branch would also be useful.
Now I use the pull request git log "Merge new-commit-id into old-commit-id" to filter commits in the PR and then do checkpatch.sh. But for master push, it should not work and skip. Just see the github action job steps.if could be used to judge it. I'll update it later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update on trigger pull request for all branches. As to push event, I think it may be not necessary since there is no committer push master directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it captures the merge situation which is what I'm worried about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is also the nightly build to make the full build although it's per day.


jobs:
check:
runs-on: ubuntu-18.04
container: liuguo09/ubuntu-nuttx:18.04

steps:
- name: Checkout nuttx repo
uses: actions/checkout@v2
with:
path: nuttx
fetch-depth: 0

- name: Checkout apps repo
uses: actions/checkout@v2
with:
repository: apache/incubator-nuttx-apps
path: apps
fetch-depth: 0

- name: Check Pull Request
run: |
cd nuttx
ranges=`git log -1 --merges --pretty=format:%P | awk -F" " '{ print $1 ".." $2 }'`
git log --oneline $ranges
commits=`git log --reverse --format=format:%H $ranges`
echo "./tools/checkpatch.sh -g $commits"
./tools/checkpatch.sh -g $commits

build:
needs: check
runs-on: ubuntu-18.04
container: liuguo09/ubuntu-nuttx:18.04

strategy:
matrix:
boards: [arm-01, arm-02, arm-03, arm-04, arm-05, arm-06, arm-07, arm-08, arm-09, arm-10, arm-11, arm-12, arm-13, arm-14, arm-15, mips-riscv-x86, sim]

steps:
- name: Checkout nuttx repo
uses: actions/checkout@v2
with:
path: nuttx
fetch-depth: 0

- name: Fetch nuttx tags
run: |
cd nuttx
git fetch --tags
Copy link
Contributor

Choose a reason for hiding this comment

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

Why need fetch tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since tags is needed to generate the .version file, or it would build break at first : (

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

Copy link
Contributor

Choose a reason for hiding this comment

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

This can be done in the checkout step https://github.com/actions/checkout#fetch-all-tags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be done in the checkout step https://github.com/actions/checkout#fetch-all-tags

Since I'm using the with option for checkout@v2 action, the run step seems not work together. I'll double check it.

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 have checked that the instructions in the https://github.com/actions/checkout#fetch-all-tags run two steps too.
So it doesn't make much sense to use same as it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the cost is low for the two steps, but other examples there like submodules show two run lines which I think makes sense to just get everything done. But these are really later optimizations.


- name: Checkout apps repo
uses: actions/checkout@v2
with:
repository: apache/incubator-nuttx-apps
path: apps
fetch-depth: 0

- name: Checkout testing repo
uses: actions/checkout@v2
with:
repository: apache/incubator-nuttx-testing
path: testing

- name: Run builds
run: |
cd testing
./cibuild.sh -x testlist/${{matrix.boards}}.dat