Skip to content

Conversation

ricardozanini
Copy link
Member

@ricardozanini ricardozanini commented Nov 30, 2023

Many thanks for submitting your Pull Request ❤️!

Please specify parts of this PR update:

  • Specification
  • Schema
  • Examples
  • Extensions
  • Roadmap
  • Use Cases
  • Community
  • TCK
  • Other

Discussion or Issue link:

#794

What this PR does / why we need it:

  • Add all the examples from examples/README.md file to separate files, so external tooling can now import them and run for tests
  • Add a Node.js application to verify our schemas against those examples
  • Add a GitHub Action to run in every PR to verify if the examples are valid to the Schemas

Open a new PR:

  • Add the properties field to each separate definition so we enable single validation. For example, functions.json should be able to serve as a single schema definition.

Special notes for reviewers:
@JBBianchi After merging this one, we can then add the git post-hook to have an updated README file.

Additional information:
Now we have an application to maintain :)
I'm not a Node.js/Typescript specialist, so if there's something to improve, please let me know in the review.

Copy link
Member

@JBBianchi JBBianchi left a comment

Choose a reason for hiding this comment

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

Regarding the TS, I think it's small enough not to bother too much. Just for informative purposes, I created a fork to use async/await, it might be better than using blocking apis and more "readable" or "accessible" to some, but not very important.

For the rest, LGTM ! Great work as usual @ricardozanini ! ❤️

@ricardozanini
Copy link
Member Author

@JBBianchi So I can expect a PR soon coming here upgrading this code base + post-hook/templating README? :D

@JBBianchi
Copy link
Member

@JBBianchi So I can expect a PR soon coming here upgrading this code base + post-hook/templating README? :D

Yes, with pleasure, I'll do as soon as than one is merged.

@ricardozanini
Copy link
Member Author

@cdavernas can you take a look? I'll open a second PR to actually solve #794.

@ricardozanini ricardozanini changed the title Fix #794 - Add Schema Validation and Examples to CI Add Schema Validation and Examples to CI Dec 1, 2023
Copy link
Member

@cdavernas cdavernas left a comment

Choose a reason for hiding this comment

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

Didn't check it all, but I had a good overview and looks great to me!

Thanks for your awesome work @ricardozanini !

@ricardozanini
Copy link
Member Author

Has been two weeks since the introduction of this PR and we have two votes for merging. I think we can go ahead @cdavernas

@cdavernas
Copy link
Member

@ricardozanini Agreed, as per code of conduct.

@cdavernas cdavernas merged commit cd157ae into serverlessworkflow:main Dec 13, 2023
@ricardozanini ricardozanini deleted the issue-794 branch December 13, 2023 15:49
gciavarrini added a commit to gciavarrini/validation-example-app that referenced this pull request Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants