Skip to content

Conversation

@yamt
Copy link
Contributor

@yamt yamt commented Jan 29, 2020

No description provided.

@yamt
Copy link
Contributor Author

yamt commented Jan 29, 2020

This would have caught a recent bug: #181

@yamt
Copy link
Contributor Author

yamt commented Jan 29, 2020

This patch is the step 4, 5 of https://docs.travis-ci.com/user/tutorial/#to-get-started-with-travis-ci
Someone with permission on this repo can do the step 1, 2, 3.

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Jan 29, 2020

This patch is the step 4, 5 of https://docs.travis-ci.com/user/tutorial/#to-get-started-with-travis-ci
Someone with permission on this repo can do the step 1, 2, 3.

We have to use Jenkins and server from Apache to conform Apache requirement:
#174
If you are interesting in setup the automation build and test, @liuguo09, @maht and you can work together.
Also, per the discussion, all CI related stuff is put here:
https://github.com/apache/incubator-nuttx-testing

@yamt
Copy link
Contributor Author

yamt commented Jan 29, 2020

even if you don't activate it on this repo, this is convenient for people working on their fork on github.

@davids5
Copy link
Contributor

davids5 commented Jan 31, 2020

What is the harm in merging this to see how well it works. Is it not completely low-risk?

#
CONFIG_ARCH="sim"
CONFIG_ARCH_BOARD="sim"
CONFIG_ARCH_BOARD_SIM=y
Copy link
Contributor

Choose a reason for hiding this comment

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

This change seems out of place with the other commits in the PR. Perhaps it should be a separate PR?

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 change is required by other changes as otherwise ./nuttx just hangs after running the tests.

@@ -0,0 +1,46 @@
#! /bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

PLEASE FIX: All .sh files reequire (currently) a BSD 3-clause license. No unlicensed shell scripts may come in.

(License will, eventually, be changed to Apache).

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 added apache license

.travis.yml Outdated
@@ -0,0 +1,17 @@
sudo: required
Copy link
Contributor

@patacongo patacongo Jan 31, 2020

Choose a reason for hiding this comment

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

Question: Do .yml files require licenses and headers? I would think the answer is YES. ??

I think that basically any that "executes" needs a license header.

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 added apache license to this file too as this yaml file contains shell script snippets.

@justinmclean
Copy link
Member

ASF policy on headers is reasonable clear [1], but if in doubt add a header. BSD headers should not be added to new files ASF ALv2 headers should be used.

  1. https://www.apache.org/legal/src-headers.html#headers

@yamt
Copy link
Contributor Author

yamt commented Feb 3, 2020

i added apache license to shell scripts

@yamt
Copy link
Contributor Author

yamt commented Feb 6, 2020

any takers? eventually i want to add windows build which i can't test locally. i guess it's possible as travis ci provides windows env.

@maht
Copy link
Contributor

maht commented Feb 6, 2020

I think is great to have this code available, but maybe it should be placed in incubator-nuttx-testing, as requested by @patacongo [1] and others.

Maybe this preference of a separate repo for the CI code is not settled on stone and it can be reconsidered, but other people then will require to put their own scripts on main repo.

Can the CircleCI script be rewritten to clone the testing repo and move the logic there? I know maybe is a PITA to move it, but maybe would be easier to be accepted, and that is what I have been doing for other CI scripts, like the Jenkins one I have under review in that repo: put a "placeholder" Jenkinsfile in main repo so the CI can "hook" to it and the actual logic in the testing one.

[1] https://www.mail-archive.com/dev@nuttx.apache.org/msg00436.html

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Feb 6, 2020

Hi @yamt , @liuguo09 will look this patch and try to integrate with cibuild.sh. Since github could:
1.Provide Linux/MacOS/Windows VM
2.The computing power is better than Apache
3.Integrate with the workflow smoothly
We are considering using github as PR check and apache as nightly build. @liuguo09 will send the current state and plan to dev@nuttx.apache.org for furture discussion.

Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 left a comment

Choose a reason for hiding this comment

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

any takers? eventually i want to add windows build which i can't test locally. i guess it's possible as travis ci provides windows env.

@yamt could you look this PR:
#261
Which provide a unified framework to the precheck for all configs in Linux env, but it design to cover macOS and Windows in mind, so you can provide the patch to enable Windows/macOS check.

@xiaoxiang781216
Copy link
Contributor

Let's track CI issue by:
#261

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.

6 participants