-
Notifications
You must be signed in to change notification settings - Fork 584
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
Add Support for Python Upgrade Rules #9776
Conversation
opts.fileSystem[fn] = ts2 | ||
if (fn.indexOf("/") == -1 && U.endsWith(fn, extension)) { | ||
const initial = opts.fileSystem[fn] | ||
const patched = pxt.patching.patchPython(version, initial) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is being used by both Typescript and Python, should this be more generic? Because if computePyPatches
returns undefined for TypeScript, that means it won't compute any upgrades, right? It would just return fileContents
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or should there just also be patchJavascript
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, good catch. I may need to split this out after all and not re-use...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, looks like this needs something like a const patchFunction = extension == ".ts" ? pxt.patching.patchJavaScript : pxt.patching.patchPython;
then call that here to me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opted to just keep them separate. Seemed simpler at this point...
webapp/src/compiler.ts
Outdated
@@ -953,6 +958,37 @@ function upgradeFromTSAsync(): Promise<UpgradeResult> { | |||
}); | |||
} | |||
|
|||
function upgradeFromPythonAsync(): Promise<UpgradeResult> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
async/await-ify this please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Need this so we can update python code when we make a breaking change. I mostly just copied whatever we were doing for typescript and tried to do it for python as well.
Need this so we can update python code when we make a breaking change. I mostly just copied whatever we were doing for typescript and tried to do it for python as well.