-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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 serialisation of findMatch action #11233
Conversation
For whatever reason the local settings model tests have decided to stop working on my machine with this error:
The DLL definitely exists at that location...
I've added a test and am fairly sure it should work, but can't confirm due to the above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WOW okay, what a silly typo. Thanks!
Meh - I'm tempted to say close that one and open a new one, but also I feel like we've got something similar already hiding on the backlog |
Tests still error out after a rebuild of all the terminal projects 😢 I've updated the PR description to close #11225. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, I wonder what happened here. All the other ToJson seem fine. Thanks for doing this!
Hello @carlos-zamora! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 2 hours 46 minutes. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
@msftbot merge this in 5 minutes |
Hello @carlos-zamora! 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:
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". |
## Summary of the Pull Request Fixes the serialisation of the findMatch action so that the direction is stored. ## PR Checklist * [x] Closes #11225 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [x] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx ## Validation Steps Performed Added a test and tested manually.
## Summary of the Pull Request Fixes the serialisation of the findMatch action so that the direction is stored. ## PR Checklist * [x] Closes #11225 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [x] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx ## Validation Steps Performed Added a test and tested manually.
🎉 Handy links: |
🎉 Handy links: |
Summary of the Pull Request
Fixes the serialisation of the findMatch action so that the direction is stored.
PR Checklist
Validation Steps Performed
Added a test and tested manually.