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 Foyle language id on cells #1389

Merged
merged 1 commit into from
May 29, 2024
Merged

Fix Foyle language id on cells #1389

merged 1 commit into from
May 29, 2024

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented May 28, 2024

@jlewi
Copy link
Contributor Author

jlewi commented May 28, 2024

Right now when I try to build this I get an error

npm ERR! invalid: @buf/stateful_runme.community_timostamm-protobuf-ts@2.9.4-20240515171834-148b5afad0df.3 /Users/jlewi/git_vscode-runme/node_modules/@buf/stateful_runme.community_timostamm-protobuf-ts

I'm guessing this is might be related to #1378 which says the proto changes need to be published.

@jlewi
Copy link
Contributor Author

jlewi commented May 28, 2024

Confirmed; I cherry-picked this change onto the 3.5.7 branch and the build succeeded so I don't think the failure is related to my PR.

@jlewi
Copy link
Contributor Author

jlewi commented May 28, 2024

I confirmed when I cherry-picked this onto the 3.5.7 it fixed jlewi/foyle#125

@jlewi jlewi marked this pull request as ready for review May 28, 2024 22:13
@sourishkrout
Copy link
Member

Confirmed; I cherry-picked this change onto the 3.5.7 branch and the build succeeded so I don't think the failure is related to my PR.

Just merged that into main. Rebasing the branch should fix it too but cherry-pick will do.

* See jlewi/foyle#125 the language id isn't being set on cells inserted from Foyle
* As a result the language gets set to `text` and they aren't runnable.
@jlewi
Copy link
Contributor Author

jlewi commented May 29, 2024

@sourishkrout I think this PR is ready for review. I rebased on main; I'm still seeing the build error.
But when I cherry-picked this onto 3.5.7 it fixed the Foyle issue with cells not be executable.

Copy link

sonarcloud bot commented May 29, 2024

@sourishkrout sourishkrout self-requested a review May 29, 2024 14:36
Copy link
Member

@sourishkrout sourishkrout left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@sourishkrout
Copy link
Member

@sourishkrout I think this PR is ready for review. I rebased on main; I'm still seeing the build error. But when I cherry-picked this onto 3.5.7 it fixed the Foyle issue with cells not be executable.

🤔 it worked for me after I ran runme run setup (which runs npm install) to pull in the latest deps.

@sourishkrout sourishkrout merged commit 80679ec into main May 29, 2024
4 checks passed
@sourishkrout sourishkrout deleted the jlewi/fixlang branch May 29, 2024 14:38
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