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

Support pulling diagnostics for notebooks too #1465

Merged
merged 8 commits into from
May 13, 2024

Conversation

rchiodo
Copy link
Contributor

@rchiodo rchiodo commented Apr 22, 2024

Fixes #1464

Please let me know if this is acceptable. Wasn't sure if formatting was correct or if I did something weird in the testing.

I also tested this using the Pylance extension to verify I do receive pull diagnostics requests for notebooks in Pylance.

@rchiodo
Copy link
Contributor Author

rchiodo commented Apr 26, 2024

@dbaeumer do you see these automatically? Sorry wasn't sure if you saw this or not.

@dbaeumer
Copy link
Member

Yes, I do but I was out sick last week :-(

@dbaeumer
Copy link
Member

Great work. I highly appreciate the effort. See my comments. I am open for discussion since I might have missed some design choices as well.

client/src/common/notebook.ts Outdated Show resolved Hide resolved
client/src/common/notebook.ts Outdated Show resolved Hide resolved
client/src/common/notebook.ts Outdated Show resolved Hide resolved
client/src/common/notebook.ts Outdated Show resolved Hide resolved
client/src/common/notebook.ts Outdated Show resolved Hide resolved
client/src/common/notebook.ts Outdated Show resolved Hide resolved
client/src/common/diagnostic.ts Outdated Show resolved Hide resolved
client/src/common/diagnostic.ts Show resolved Hide resolved
client/src/common/notebook.ts Outdated Show resolved Hide resolved
@dbaeumer
Copy link
Member

dbaeumer commented May 8, 2024

Looks good to me. Only one small request for change.

@dbaeumer dbaeumer enabled auto-merge (squash) May 13, 2024 10:26
@dbaeumer
Copy link
Member

@rchiodo thanks again for the great PR.

@VSCodeTriageBot VSCodeTriageBot added this to the May 2024 milestone May 13, 2024
@dbaeumer dbaeumer merged commit 4fbd145 into microsoft:main May 13, 2024
6 checks passed
@rchiodo rchiodo deleted the rchiodo/notebook_diag_publish branch May 13, 2024 16:09
Copy link

@LLotme LLotme left a comment

Choose a reason for hiding this comment

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

..

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.

Diagnostics pulling doesn't work for notebook cells
5 participants