Skip to content

doc: clean up the schema#5304

Merged
DHowett-MSFT merged 8 commits intomasterfrom
dev/cazamor/schema/nullable-colors
Apr 28, 2020
Merged

doc: clean up the schema#5304
DHowett-MSFT merged 8 commits intomasterfrom
dev/cazamor/schema/nullable-colors

Conversation

@carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Apr 9, 2020

Summary of the Pull Request

This PR performs a number of miscellaneous clean-up tasks on our schema:

  • allow null for some settings
  • update some default values to be more useful
  • consistently use " instead of '
  • remove colorTable
  • provide missing description for backgroundImageOpacity
  • update cursorShape to have options match order displayed in vs code
  • if a setting has multiple options, provide a list of those options with a short description on each
    • this one I only did a few times
  • remove a little bit of ambiguity on some descriptions

PR Checklist

Closes #5279

Detailed Description of the Pull Request / Additional comments

Validation was performed for...

  • Global Settings
  • Profile Settings
  • Keybinding Actions

Validation Steps Performed

I linked VS Code to the my version of the schema. Defined all settings from scratch in settings.json. Anytime I found something off, I updated the schema and saw if it looks right.

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Apr 9, 2020
@zadjii-msft zadjii-msft added the Area-Schema Things that have to do with the json schema. label Apr 10, 2020
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Apr 10, 2020
@carlos-zamora
Copy link
Member Author

As a general question, when we provide a list of possible values in the schema, should we list the default value? I could honestly go either way.

@carlos-zamora carlos-zamora marked this pull request as ready for review April 10, 2020 20:55
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

I think I'm fine with this but I'm not a wordsmith so I'll the them be the official sign-offs

@carlos-zamora carlos-zamora force-pushed the dev/cazamor/schema/nullable-colors branch from 6186fe0 to 84ebd21 Compare April 17, 2020 20:25
@cinnamon-msft cinnamon-msft added the Needs-Second It's a PR that needs another sign-off label Apr 28, 2020
@ghost ghost requested review from leonMSFT and miniksa April 28, 2020 18:21
@DHowett-MSFT DHowett-MSFT changed the title Clean up schema doc: clean up the schema Apr 28, 2020
@DHowett-MSFT DHowett-MSFT merged commit 9e23ee7 into master Apr 28, 2020
@DHowett-MSFT DHowett-MSFT deleted the dev/cazamor/schema/nullable-colors branch April 28, 2020 19:14
DHowett-MSFT pushed a commit that referenced this pull request Apr 28, 2020
## Summary of the Pull Request
This PR performs a number of miscellaneous clean-up tasks on our schema:
- [X] allow null for some settings
- [X] update some default values to be more useful
- [X] consistently use " instead of '
- [X] remove colorTable
- [X] provide missing description for `backgroundImageOpacity`
- [X] update cursorShape to have options match order displayed in vs code
- [X] if a setting has multiple options, provide a list of those options with a short description on each
   - this one I only did a few times
- [X] remove a little bit of ambiguity on some descriptions

## PR Checklist
Closes #5279

## Detailed Description of the Pull Request / Additional comments
Validation was performed for...
- [X] Global Settings
- [X] Profile Settings
- [x] Keybinding Actions

## Validation Steps Performed
I linked VS Code to the my version of the schema. Defined all settings from scratch in settings.json. Anytime I found something off, I updated the schema and saw if it looks right.

(cherry picked from commit 9e23ee7)
@ghost
Copy link

ghost commented Apr 28, 2020

🎉Windows Terminal Preview v0.11.1191.0 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Schema Things that have to do with the json schema. Needs-Second It's a PR that needs another sign-off

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow null for some color settings in the schema

4 participants