Feature/json schema for workflows #159
Conversation
|
@elviskahoro, it's not convenient to wait for someone's workflow approval. It's slows down my work: I can't send any test commits to check how CI works. Maybe it is worth considering disabling this constraint? |
|
Everything seems to work now. ;) |
|
@EmilyGraceSeville7cf hey emily! I've been using this project to test github actions locally: |
|
What about to review this PR? ;) There are conflicts I can't resolve due to lack of write access. |
|
JSON schemas for Warp are available here now. PR is closed as I didn't get any response from project maintainers in a reasonable time. |
|
Hey @EmilyGraceSeville7cf, jumping in here. I'm happy to review and get this over the finish line, we very much appreciate the contribution. |
|
Lmk if you'd be willing to reopen the PR and I can take a pass :) |
There was a problem hiding this comment.
Thank you so much for your work on this @EmilyGraceSeville7cf! This is a huge improvement and it's much appreciated ⚡
| "yaml.schemas": { | ||
| "schemas/workflow.json": "specs/*/*.yaml" | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: could we end all files with a new line
| "properties": { | ||
| "name": { | ||
| "title": "name", | ||
| "description": "A name of the current workflow\nhttps://github.com/EmilySeville7cfg/warp-workflows/blob/main/FORMAT.md#name", |
There was a problem hiding this comment.
Would you mind updating all of these links to point at the warpdotdev repo instead of your fork of this repo?https://github.com/warpdotdev/workflows/blob/main/FORMAT.md
| }, | ||
| "source_url": { | ||
| "title": "source url", | ||
| "description": "A source url of the current workflow\nhttps://github.com/EmilySeville7cfg/warp-workflows/blob/main/FORMAT.md#source_url", |
There was a problem hiding this comment.
Would you mind using the description that was originally used in the FORMAT.md file? "The URL from where the Workflow was originally generated from. This is surfaced in commands.dev for attribution purposes"
| } | ||
| }, | ||
| "additionalProperties": false | ||
| } |
There was a problem hiding this comment.
could we also end this file with a newline?
| "enum": [ | ||
| "zsh", | ||
| "bash", | ||
| "fish" |
There was a problem hiding this comment.
The existing format accepts either zsh or Zsh
workflows/workflow-types/src/lib.rs
Lines 5 to 10 in 495b5cf
I think we should make that explicit in this format because there could be existing workflow yaml files in the wild that use the uppercase format.
| "description": { | ||
| "title": "description", | ||
| "description": "A description of the current workflow\nhttps://github.com/EmilySeville7cfg/warp-workflows/blob/main/FORMAT.md#description", | ||
| "$ref": "#/definitions/nullable-string" |
There was a problem hiding this comment.
I'm by no means a JSON schema expert, but could you clarify why we need the nullable-string definition here? Why can't we use JSON schema's support for Required properties https://json-schema.org/understanding-json-schema/reference/object.html#required-properties
| green='\e[32m' | ||
| cyan='\e[36m' | ||
|
|
||
| install_dependency() { |
There was a problem hiding this comment.
nit: Would you mind documenting each argument to this function?
|
|
||
| - name: Run jsonschema-cli validate | ||
| run: | | ||
| for file in specs/*/*.yaml; do |
There was a problem hiding this comment.
Could we support both yaml and yml? IMO we should be flexible and support both (this is what Github actoins does as well, fwiw https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#about-yaml-syntax-for-workflows)
| @@ -23,3 +32,6 @@ cargo install cargo-diff-tools --git https://github.com/warpdotdev/cargo-diff-to | |||
| brew update | |||
|
|
|||
| rustup component add clippy | |||
|
|
|||
| install_dependency python3-pip 'Python package manager' 'sudo apt install python3-pip' | |||
There was a problem hiding this comment.
I think we should avoid sudo apt install here, the current script/bootstrap is intended to run on Mac (which is why we install brew).
Also, I'm curious your thoughts on whether we should use https://www.npmjs.com/package/ajv-cli instead. It's the one of the official CLI JSON validators mentioned by the JSON schema org https://json-schema.org/implementations.html.
Discord username (optional) please include so we can attribute you with our Contributor role (like so elvis#4747)
Emily Grace Seville#9556Description of changes (updated or new workflows)