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.

Sign up for GitHub

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

Jump to bottom

Allow profile.padding to be an int (or any other type :|) #7235

Merged
1 commit merged into from
Aug 10, 2020
Merged

Allow profile.padding to be an int (or any other type :|) #7235

1 commit merged into from
Aug 10, 2020

Conversation

DHowett
Copy link
Member

@DHowett DHowett commented Aug 10, 2020

Summary of the Pull Request

We're expecting that people have treated padding as an integer, and the type-based converter is too strict for that. This PR widens its scope and explicitly allows for it in the schema.

PR Checklist

@ghost ghost added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree. labels Aug 10, 2020
@zadjii-msft zadjii-msft added the Needs-Second It's a PR that needs another sign-off label Aug 10, 2020
@zadjii-msft
Copy link
Member

the nitpicker in me wants a unittest as well, but I want the crash fixed more

@DHowett
Copy link
Member Author

DHowett commented Aug 10, 2020

@msftbot merge this in 1 minute

@ghost ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 10, 2020
@ghost
Copy link

ghost commented Aug 10, 2020

Hello @DHowett!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Mon, 10 Aug 2020 19:48:07 GMT, which is in 1 minute

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@ghost ghost merged commit e6c71cb into master Aug 10, 2020
@ghost ghost deleted the dev/duhowett/padding branch August 10, 2020 19:48
@cinnamon-msft cinnamon-msft mentioned this pull request Aug 20, 2020
Closed
14 tasks
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlos-zamora carlos-zamora carlos-zamora approved these changes

@zadjii-msft zadjii-msft zadjii-msft approved these changes

@miniksa miniksa Awaiting requested review from miniksa

@leonMSFT leonMSFT Awaiting requested review from leonMSFT

@PankajBhojwani PankajBhojwani Awaiting requested review from PankajBhojwani

Assignees
No one assigned
Labels
Area-Settings Issues related to settings and customizability, for console or terminal AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Second It's a PR that needs another sign-off Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Providing an int value for padding will hyperexplode the Terminal on launch
3 participants
@DHowett @zadjii-msft @carlos-zamora