Skip to content

Conversation

@javirln
Copy link
Member

@javirln javirln commented Aug 29, 2024

This patch adds a plugin to generate json schemas based on the proto defines in the code. Since we are only interested in the ones about workflowcontracts, I've modified the Make file to get rid of those unwanted files.

Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
@javirln javirln requested review from jiparis and migmartri August 29, 2024 10:54
@javirln javirln self-assigned this Aug 29, 2024
cd ./plugins/sdk/v1/plugin/api && buf generate
cd ./api && buf generate
# Remove all json schema that are not part of the workflowcontract since we don't need them
find ./api/gen/jsonschema -type f ! -name 'workflowcontract.*' -exec rm {} +
Copy link
Member

Choose a reason for hiding this comment

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

mm, I am not sure this is correct. in the CI we might be running generation without makefile, and afaik we are not enforcing using make for things

What's wrong with having all the files? How many are those?

Copy link
Member Author

Choose a reason for hiding this comment

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

There are 189 generated files without the option of not generating them.

- outputClientImpl=grpc-web # client implementation it generates
- esModuleInterop=true # use imports as required in modern ts setups
- useOptionals=messages # use optional TypeScript properties instead of undefined
- plugin: buf.build/bufbuild/protoschema-jsonschema
Copy link
Member

Choose a reason for hiding this comment

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

can you make sure we ignore them here too?

https://github.com/chainloop-dev/chainloop/blob/main/.gitattributes

Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
Copy link
Member

@migmartri migmartri left a comment

Choose a reason for hiding this comment

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

LGTM!

@javirln javirln merged commit 894d6f3 into chainloop-dev:main Aug 30, 2024
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.

2 participants