Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Workflow for ZAP templates generation #3929

Merged

Conversation

jepenven-silabs
Copy link
Contributor

Problem

Since the ZAP templates are now within the CHIP repo, we needed a way to make sure that no could can break them.

Summary of Changes

Added a GitHub workflow to generate all the files needed for the all_clusters_app since it's the most complex configuration that we have. This workflow is an adaptation of the one found in the ZAP repo. Unfortunately node wasn't able to run on the chip-build container. Probably because of permission issues ( Issue #3928)

Fixes #3885

@jepenven-silabs
Copy link
Contributor Author

@mspang @BroderickCarlin @andy31415 could we speed up the merge of this PR ? Sooner the CI is in master, the less issues will have with templates updates.

@andy31415
Copy link
Contributor

andy31415 commented Nov 24, 2020

@mspang @BroderickCarlin @andy31415 could we speed up the merge of this PR ? Sooner the CI is in master, the less issues will have with templates updates.

Sorry for the delay. I am trying to figure out the long term plan for this - I expect at some point we will do generation as part of the build rather than separately.

Is this PR intended as a test to validate that we can always 'zap gen' and will go away once we automatically generate?

Also, any chance you could rebase to fix the cirque failure?

@jepenven-silabs
Copy link
Contributor Author

@mspang @BroderickCarlin @andy31415 could we speed up the merge of this PR ? Sooner the CI is in master, the less issues will have with templates updates.

Sorry for the delay. I am trying to figure out the long term plan for this - I expect at some point we will do generation as part of the build rather than separately.

Is this PR intended as a test to validate that we can always 'zap gen' and will go away once we automatically generate?

Also, any chance you could rebase to fix the cirque failure?

Yup. This CI is mainly to prevent people from 2 things :

  • Adding errors in templates (non terminated iterator, bad helper spelling, missing }, etc )
  • Downgrading the zap submodule. Since most of the helper used are tightly bound with a specific commit of zap, we don't want someone to accidentally downgrade zap and thus breaking the templates generation.

In the future this will be a pre-build step for every build CI job. However this cannot be for now for two simple reasons.

  • All of the required templates are not yet complete (endpoint_config is one of them)
  • Running npm install inside the chip-build container fails because of permission problems.

Since the templates are not complete and there's still some issues that need to be address with the templates already present in the repo, it doesn't make sense for now to work on adding the zap gen step to all of the build CI. Hence this PR.

.github/workflows/zap_templates.yaml Outdated Show resolved Hide resolved
@jepenven-silabs
Copy link
Contributor Author

@andy31415 @mspang Can we merge this now ?

@jepenven-silabs jepenven-silabs self-assigned this Dec 7, 2020
@andy31415 andy31415 merged commit 0998211 into project-chip:master Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ZAP templates generation to CI
7 participants