Skip to content

hotfix vsc extension may cause kernel to crash#118

Merged
philipnickel merged 4 commits into
dtudk:mainfrom
philipnickel:hotfix/vscPdf-extension-issues
Oct 22, 2025
Merged

hotfix vsc extension may cause kernel to crash#118
philipnickel merged 4 commits into
dtudk:mainfrom
philipnickel:hotfix/vscPdf-extension-issues

Conversation

@philipnickel
Copy link
Copy Markdown
Member

Manuel discovered this. Figured it's save to just avoid installing the extension since it's a 'nice to have' anyway

Philip Korsager Nickel added 2 commits October 21, 2025 16:11
Manuel discovered this. Figured it's save to just avoid installing the
extension since it's a 'nice to have' anyway
Copy link
Copy Markdown
Contributor

@zerothi zerothi left a comment

Choose a reason for hiding this comment

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

So you're saying that the pdf extension might create crashes to the kernel... WTF?

Nice you discovered this!

Should we warn the others about this, trying to remove that extension and see if kernel crashes disappear?

@zerothi
Copy link
Copy Markdown
Contributor

zerothi commented Oct 22, 2025

Do you know why it fails on windows? Is it related to your change?

@zerothi
Copy link
Copy Markdown
Contributor

zerothi commented Oct 22, 2025

I don't see why it does the same line 3 times... huh...

@philipnickel
Copy link
Copy Markdown
Member Author

I don't see why it does the same line 3 times... huh...

The wonders of github raw caching maybe....

@zerothi
Copy link
Copy Markdown
Contributor

zerothi commented Oct 22, 2025

So I can safely merge it? 🤪

@philipnickel
Copy link
Copy Markdown
Member Author

Do you know why it fails on windows? Is it related to your change?

I think it's the caching.. I'll make a small change and trigger a refresh. That sometimes works.
That's part of why we should figure out how do avoid depending on curling from github raw. It's a pain in development and very unpredictable

@zerothi
Copy link
Copy Markdown
Contributor

zerothi commented Oct 22, 2025

I can just press replay, I'll try!

@zerothi
Copy link
Copy Markdown
Contributor

zerothi commented Oct 22, 2025

yeah, it came through!

@philipnickel
Copy link
Copy Markdown
Member Author

So you're saying that the pdf extension might create crashes to the kernel... WTF?

Nice you discovered this!

Should we warn the others about this, trying to remove that extension and see if kernel crashes disappear?

I have no idea how those things even interact with each other. Manuel claimed it fixed the issue while debugging on a students pc. Maybe it's best we stick with only official extensions as a baseline and then discuss if there should be optional 'nice-to-haves' that can be quickly reverted.

@zerothi
Copy link
Copy Markdown
Contributor

zerothi commented Oct 22, 2025

Could you write out on the channel in #common-problems about this? Thanks! (just when you have time!)

@zerothi
Copy link
Copy Markdown
Contributor

zerothi commented Oct 22, 2025

yeah, you don't need to repush if it's a cache issue, an owner of a repo can press retry, for next time ;)

Comment thread Windows/Components/VSC/install.ps1 Outdated
@@ -118,7 +118,7 @@ Write-Host "Installing VSCode extensions..."
$extensions = @(
"ms-python.python",
"ms-toolsai.jupyter",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

uh, this breaks, no trailing comma allowed!

@zerothi
Copy link
Copy Markdown
Contributor

zerothi commented Oct 22, 2025

Great, could you remove the 2nd commit?

@philipnickel philipnickel merged commit 953d774 into dtudk:main Oct 22, 2025
6 checks passed
@philipnickel
Copy link
Copy Markdown
Member Author

Great, could you remove the 2nd commit?

Ohhh missed it.. what's the correct thing to do now? Reverting the merge?

@zerothi
Copy link
Copy Markdown
Contributor

zerothi commented Oct 22, 2025

No, the branch is protected, so one cannot do this.

You'll generally (in these cases) want to do a squash merge, because that will remove fixing commits which are just annoying in the history.

So there are 2 ways:

  1. use the pull request to finalize the commits, then afterwards one will do a manual rebase which will squash the commits that you don't want in the history
  2. Or, finalize all things on this PR, then do a squash merge and correct the message.

In 2. you are reduced to always 1 commit, which is typically very good for these small changes here.
In 1. you have the ability to re-arrange commits, but also remove unnecessary ones. This is more flexible.

We won't do anything now.

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