You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/09/13 17:00:02 UTC

[GitHub] [incubator-nuttx-apps] v01d opened a new pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

v01d opened a new pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382


   ## Summary
   
   Follow up change to https://github.com/apache/incubator-nuttx/pull/1771
   
   ## Impact
   
   CI
   
   ## Testing
   
   CI


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] v01d commented on pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382#issuecomment-692186099


   I'd be in favor of having one less repo to worry about and tie into the workflows. It would only add scripts inside `tools/` and inside `.github` so I think it does not bother to have it there.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382#issuecomment-691699976


   One more for testing?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382#issuecomment-692185034


   @xiaoxiang781216 I'm open to whatever on this, I mostly just wanted to point out that what is in that repo will likely grow as the testing infra matures a bit and I think the pain we are currently facing will get better.  I know there was also a fair bit of push back last time about moving CI stuff into the OS repo.  As I said I'm mostly indifferent as `.github/workflows` is mostly shared anyway. The action could probably be left in place.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton merged pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

Posted by GitBox <gi...@apache.org>.
btashton merged pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382#issuecomment-692015661


   Sure, I will open a thread.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382#issuecomment-692179992


   > > Sure, I will open a thread.
   > 
   > @xiaoxiang781216 I'm inclined to not because if we do create some GitHub Actions via nodejs they will add quite a bit of code that does not really need to be in the OS. We will also pay a fairly large cost cloning the repo because of the OS repo size.
   > 
   
   The automation build script is composed by two part:
   
   1. The core script(cibuild.sh, *.dat and Dockfile)
   2. The github specific script(under .github/)
   
   The first category is very small and reusable in any environment. My suggestion is moving this category to nuttx/tools.
   
   > This also still does not solve having to keep the workflows in sync with apps.
   > 
   > The fact that it is on the public roadmap to share workflows across repos makes me hopeful
   
   The second category which tightly couple with the github workflow and duplicate in three repos keep as the current state until you have to time to optimize them to the perfect shape.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] davids5 commented on pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382#issuecomment-691979848


   >How about we merge incubator-nuttx-testing into nuttx/tools which will simplify the furture build enhancement a lot. BTW, it's also very helpful for people who want to setupt the local CI infrastructure.
   
   @xiaoxiang781216 and PPMC  Is there any objection? Do we need to add a [DISCUSS] and then vote on it?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382#issuecomment-692129906


   > Sure, I will open a thread.
   
   @xiaoxiang781216 I'm inclined to not because if we do create some GitHub Actions via nodejs they will add quite a bit of code that does not really need to be in the OS. We will also pay a fairly large cost cloning the repo because of the OS repo size.
   
   This also still does not solve having to keep the workflows in sync with apps. 
   
   The fact that it is on the public roadmap to share workflows across repos makes me hopeful


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382#issuecomment-692207390


   > > This also still does not solve having to keep the workflows in sync with apps.
   > > The fact that it is on the public roadmap to share workflows across repos makes me hopeful
   > 
   > The second category which tightly couple with the github workflow and duplicate in three repos, still keep as the current state until you have to time to optimize them to the perfect shape.
   
   For reference this is the roadmap item https://github.com/github/roadmap/issues/98


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 edited a comment on pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382#issuecomment-692179992


   > > Sure, I will open a thread.
   > 
   > @xiaoxiang781216 I'm inclined to not because if we do create some GitHub Actions via nodejs they will add quite a bit of code that does not really need to be in the OS. We will also pay a fairly large cost cloning the repo because of the OS repo size.
   > 
   
   The automation build script is composed by two part:
   
   1. The core script(cibuild.sh, *.dat and Dockfile)
   2. The github specific script(under .github/)
   
   The first category is very small and reusable in any environment. My suggestion is moving this category to nuttx/tools.
   
   > This also still does not solve having to keep the workflows in sync with apps.
   > 
   > The fact that it is on the public roadmap to share workflows across repos makes me hopeful
   
   The second category which tightly couple with the github workflow and duplicate in three repos, still keep as the current state until you have to time to optimize them to the perfect shape.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382#discussion_r487621138



##########
File path: .github/workflows/build.yml
##########
@@ -180,7 +180,7 @@ jobs:
         with:
           run: |
             cd sources/testing
-            ./cibuild.sh -x testlist/${{matrix.boards}}.dat
+            ./cibuild.sh testlist/${{matrix.boards}}.dat

Review comment:
       @v01d need modfiy these too:
   https://github.com/apache/incubator-nuttx-testing/blob/master/.github/workflows/build.yml#L84
   https://github.com/apache/incubator-nuttx-testing/blob/master/.github/workflows/build.yml#L142
   How about we merge incubator-nuttx-testing into nuttx/tools which will simplify the furture build enhancement a lot. BTW, it's also very helpful for people who want to setupt the local CI infrastructure.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] davids5 commented on pull request #382: Remove -x from cibuild.sh call in CI since this will abort the build on first error and we do not want that

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #382:
URL: https://github.com/apache/incubator-nuttx-apps/pull/382#issuecomment-692205351


   > I know there was also a fair bit of push back last time about moving CI stuff into the OS repo.
   
   @btashton 
   
   I was being sensitive to that past position, this is why I asked for discuss on it. Now that we have all had some time using GH we may be in a new place of understanding and feel different about it.  If the answer is no, that is fine as well.  But I for one would like to set up a test server and be able to help with the CI (without breaking it :) ) 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org