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

fix(json-schema): add missing platforms property to cmds for #1915

Merged
merged 2 commits into from
Nov 15, 2024

Conversation

dkarter
Copy link
Contributor

@dkarter dkarter commented Nov 11, 2024

Motivation

I've been getting errors from my editor that using platforms as a sibling of for under cmds is not supported, when it is in fact supported and respected by task

I believe this was missed on #980

Changes

  • This change simply adds a small fix to the json schema to denote that
    platforms is allowed with for

Screenshots / Testing Instructions

🖼️ Before
CleanShot 2024-11-10 at 19 35 59@2x

🖼️ After
CleanShot 2024-11-10 at 19 35 34@2x

@dkarter dkarter marked this pull request as ready for review November 11, 2024 01:36
@dkarter dkarter changed the title fix: add missing platforms property to cmds for fix(json-schema): add missing platforms property to cmds for Nov 11, 2024
@vmaerten vmaerten self-requested a review November 11, 2024 13:18
Copy link
Member

@vmaerten vmaerten left a comment

Choose a reason for hiding this comment

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

First of all, thanks for this!
Small comment, duplicate 3 times this key is not the way to go. Can you declare it under definitions then use it as a ref ?

like :

        "platforms": {
          "ref": "#/definitions/platforms"
        }

@dkarter
Copy link
Contributor Author

dkarter commented Nov 14, 2024

@vmaerten great suggestion! Fixed in 4065c3e

@vmaerten vmaerten merged commit 517bb3f into go-task:main Nov 15, 2024
14 checks passed
@vmaerten
Copy link
Member

Thanks

vmaerten added a commit that referenced this pull request Nov 15, 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.

3 participants