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

feat: generate json schema for func.yaml #460

Merged
merged 1 commit into from
Aug 9, 2021

Conversation

zroubalik
Copy link
Contributor

@zroubalik zroubalik commented Aug 5, 2021

Signed-off-by: Zbynek Roubalik zroubali@redhat.com

Changes

  • 🎁 Generate json schema for func.yaml

/kind enhancement

Relates: #336

We can generate json schema for func.yaml, the schema could be added to https://www.schemastore.org/json/ -> most of IDEs uses schemastore to enable code completion & validation of certain files for users out of the box:

schema.mp4

I wasn't able to add validation to Envs and Labels, because they internally share the same type Pairs, but the validation of values is different for each type: https://github.com/knative-sandbox/kn-plugin-func/blob/818ec572aa52f12a35bb6b022c2e727ce6ebbfd3/config.go#L120-L123
A solution would be to re-introduce separate Go types for each property.

Release Note

Generate json schema for func.yaml

@knative-prow-robot knative-prow-robot added kind/enhancement approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 5, 2021
@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Aug 5, 2021
@knative-prow-robot knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 5, 2021
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
Copy link
Member

@lkingland lkingland left a comment

Choose a reason for hiding this comment

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

This is sure to be a much-appreciated addition by folks editing the func.yaml directly

👍🏻

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 9, 2021
@knative-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lkingland, zroubalik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [lkingland,zroubalik]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot merged commit 8939f89 into knative:main Aug 9, 2021
@zroubalik
Copy link
Contributor Author

I wasn't able to add validation to Envs and Labels, because they internally share the same type Pairs, but the validation of values is different for each type:

https://github.com/knative-sandbox/kn-plugin-func/blob/818ec572aa52f12a35bb6b022c2e727ce6ebbfd3/config.go#L120-L123

A solution would be to re-introduce separate Go types for each property.

@lance @matejvasek what do you think about this?

@lance
Copy link
Member

lance commented Aug 9, 2021

@zroubalik I guess I'm not really opposed to this. Of course, I think it's always best to have common/reusable code where possible. But validation is really nice.

@zroubalik
Copy link
Contributor Author

@zroubalik I guess I'm not really opposed to this. Of course, I think it's always best to have common/reusable code where possible. But validation is really nice.

Yeah, I share the same concern. But in this case it makes sense, this makes me wonder, whether I should make to labels as well, currently they are name&value, but more correct syntax is imho key & value, that would be a breaking change though.

@lkingland
Copy link
Member

lkingland commented Aug 9, 2021

I vote for key usually, because Name implies a standalone value, whereas Key an index into a structure that uniquely identifies the value that goes with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. kind/enhancement lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants