sdk-go
sdk-go copied to clipboard
adding validate to flows against schema
Signed-off-by: LiHeng [email protected]
Many thanks for submitting your Pull Request :heart:!
What this PR does / why we need it: need a way to validate the schemas, with this in place the remaining items would just be to validate whether the functionref/eventref/error ref is listed on top in the same workflow
Special notes for reviewers:
Additional information (if needed):
CI will fail for now as quite alot of our specs are still in v0.7, raising this to collect feedback
Hi, thanks for sending this. However I have two questions.
Why have you added vendor and, is there a use case to use this validation?
Vendor was added by mistake, will remove it in the next commit.
As for the use case, the first one is to ensure that the workflows in the repos are up to date to the latest JSON schema using that as the source of truth(can be added to CI), the second one being a utility for people who uses this package to validate their existing workflow e.g. when updating from v7 to v8 or to check the workflow they write are correct
Thanks, @brlala, but I don't think we should copy the schemas to this repo. Let's try to create our tooling (maybe in Makefile at first) to download the schemas from the website/specificationn repo (using the correct tag), then run the tests.
Thanks!
Sure, the reason to add them into the repo is prevent the files being downloaded everytime for CI, so should I continue adding the test feature for this PR?
Yes! It would be nice to have these tests for sure, but the schemas we should download them from the spec. Won't take that long. We are trying to avoid duplicating the schemas in the SDKs.
this PR could address this issue https://github.com/serverlessworkflow/sdk-go/issues/28 :)
dependencies against https://github.com/serverlessworkflow/specification/pull/707 conflicting state
Changes looks good. one thing missing, maybe you can also add the GHA that will run this test.
sure! Will work on it, is it better to run it against the latest scheme(main) branch or the latest major release(0.8)?
Changes looks good. one thing missing, maybe you can also add the GHA that will run this test.
sure! Will work on it, is it better to run it against the latest scheme(main) branch or the latest major release(0.8)?
That's a good question, maybe LATEST, PREVIOUS and main branch?
Changes looks good. one thing missing, maybe you can also add the GHA that will run this test.
sure! Will work on it, is it better to run it against the latest scheme(main) branch or the latest major release(0.8)?
That's a good question, maybe LATEST, PREVIOUS and main branch?
Just to double confirm, are you referring to LATEST - v0.8 MAIN - main PREVIOUS - v0.7
Wouldn't the CI against v0.7 always fail since some of the keywords(e.g. start
) became optional in v0.8?
Changes looks good. one thing missing, maybe you can also add the GHA that will run this test.
sure! Will work on it, is it better to run it against the latest scheme(main) branch or the latest major release(0.8)?
That's a good question, maybe LATEST, PREVIOUS and main branch?
Just to double confirm, are you referring to LATEST - v0.8 MAIN - main PREVIOUS - v0.7
Wouldn't the CI against v0.7 always fail since some of the keywords(e.g.
start
) became optional in v0.8?
Something like this. the v08 should work with v07 and v08 workflow definitions. Maybe, instead have three checks we can reorganize the examples and have them segregated by spec version. and for any new spec that is released, we check it with all the previous ones.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.