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 for schema sequences #197

Merged
merged 1 commit into from
Nov 15, 2019
Merged

Fix for schema sequences #197

merged 1 commit into from
Nov 15, 2019

Conversation

JPinkney
Copy link
Contributor

@JPinkney JPinkney commented Nov 2, 2019

Fixes: #196

The idea of this PR is described in: #81 (comment)

cc @andxu @itowlson

@coveralls
Copy link

coveralls commented Nov 2, 2019

Coverage Status

Coverage increased (+0.8%) to 67.327% when pulling 6dc9a60 on fix-schema-sequence into c1c91bd on master.

@itowlson
Copy link

itowlson commented Nov 2, 2019

Thanks @JPinkney - it's the weekend here but I'll take a look on Monday. Thanks heaps for the quick turnaround!

@itowlson
Copy link

itowlson commented Nov 4, 2019

I don't feel very qualified to comment on the code in detail, but it definitely looks like a solid approach and the implementation looks good. I'll install a local build and see if fixes the downstream problem with the k8s extension. Thanks!

@andxu
Copy link
Contributor

andxu commented Nov 4, 2019

LGTM

@itowlson
Copy link

itowlson commented Nov 4, 2019

I'm not sure if I've got something wrong in the build chain (yaml-language-server -> vscode-yaml -> vscode-kubernetes-tools) but this still doesn't seem to be working for me when I build it from source. I do still get intellisense on single documents but nothing on multi-documents.

@itowlson
Copy link

itowlson commented Nov 4, 2019

I'm not sure how to track down the problem or help give any direction given how indirect the chain is - can't think of anything except sprinkling printfs and that doesn't seem very efficient... @JPinkney let me know if there's anything I can do to help validate whether there's a problem and if so to diagnose it.

@JPinkney
Copy link
Contributor Author

JPinkney commented Nov 5, 2019

@andxu @itowlson Can you try again now, it wasn't resolving $ref properly before

@itowlson
Copy link

itowlson commented Nov 6, 2019

Looking good now! I'm using @andxu's test pod/deployment YAML and the hints and errors are looking appropriate to me. Thanks!

@andxu
Copy link
Contributor

andxu commented Nov 6, 2019

@itowlson I tested the new code and it works now

@itowlson
Copy link

@JPinkney Is there a timeframe for getting this into a release please?

@JPinkney
Copy link
Contributor Author

@itowlson I'll try for Friday, I'm a little short on time since I'm preparing for KubeCon

@itowlson
Copy link

Terrific - thanks @JPinkney!

@JPinkney JPinkney merged commit 700db67 into master Nov 15, 2019
@evidolob evidolob deleted the fix-schema-sequence branch August 11, 2020 13:19
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.

Schema Sequence is broken in 0.5.x
4 participants