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

InfoPanel generic parameter formatting & tests #870

Merged
merged 7 commits into from
Jan 21, 2022

Conversation

jcmrva
Copy link
Contributor

@jcmrva jcmrva commented Dec 17, 2021

Fix for ionide/ionide-vscode-fsharp#1583

I'll improve the tests.

@baronfel
Copy link
Contributor

Hey @jcmrva, are you ready for this to be merged? it's in draft state right now so I didn't want to go forward with it until you were ready.

@jcmrva
Copy link
Contributor Author

jcmrva commented Jan 21, 2022

Sorry, I meant to get back to this. Ideally, to make the test good, we'd want to parse the json and then the html. Let me know if you think it's worth holding it for that.

@baronfel baronfel marked this pull request as ready for review January 21, 2022 03:35
@baronfel
Copy link
Contributor

@jcmrva excellent! I added json parsing of the expected responses and your tests are green locally 👍 HTML parsing would be quite a heavy additional load IMO, so I'm gonna let the CI pipeline run and merge it.

Thanks for your efforts here, I'm hoping to release this change this weekend!

@baronfel baronfel force-pushed the infopanel-formattest branch from 5903b2d to 2aa6b3e Compare January 21, 2022 03:59
@baronfel
Copy link
Contributor

aha, the move to the Ionide.LanguageServerProtocol nuget changed the expected type names, hence the sudden build breaks

@baronfel baronfel merged commit d693d98 into ionide:main Jan 21, 2022
@baronfel
Copy link
Contributor

This was released in FSAC 0.50.0 just now, and I hope to update ionide itself over the next couple days.

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.

2 participants