Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new machine learning capability to Source Academy by integrating a Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new torch bundle, which wraps the @veehz/torch machine learning library. The bundle includes its own package.json, tsconfig.json, and an index.ts that re-exports the external library, along with an empty manifest.json indicating it's a library-only bundle. The yarn.lock file has been updated to reflect these new dependencies. Feedback from the review highlights two main points: the absence of unit tests for the new torch module, which are crucial for verifying functionality and preventing regressions, and the presence of an unnecessary serve script and its description in package.json for a library-only bundle, suggesting it should be removed for cleaner configuration.
| * @author Vee Hua Zhi | ||
| */ | ||
|
|
||
| export * from '@veehz/torch'; |
There was a problem hiding this comment.
| "scripts": { | ||
| "build": "buildtools build bundle .", | ||
| "lint": "buildtools lint .", | ||
| "tsc": "buildtools tsc .", | ||
| "test": "buildtools test --project .", | ||
| "postinstall": "buildtools compile", | ||
| "serve": "yarn buildtools serve" | ||
| }, | ||
| "scripts-info": { | ||
| "build": "Compiles the given bundle to the output directory", | ||
| "lint": "Runs ESlint on the code in the bundle", | ||
| "serve": "Starts the modules server", | ||
| "test": "Runs unit tests defined for the bundle", | ||
| "tsc": "Runs the Typescript compiler and produces the library version of the bundle" | ||
| } |
There was a problem hiding this comment.
The serve script and its corresponding description in scripts-info seem unnecessary for this bundle. Since torch is a library-only bundle without any UI components (as indicated by the empty tabs array in manifest.json), there's nothing to serve. Removing unused scripts helps keep the configuration clean and avoids confusion.
"scripts": {
"build": "buildtools build bundle .",
"lint": "buildtools lint .",
"tsc": "buildtools tsc .",
"test": "buildtools test --project .",
"postinstall": "buildtools compile"
},
"scripts-info": {
"build": "Compiles the given bundle to the output directory",
"lint": "Runs ESlint on the code in the bundle",
"test": "Runs unit tests defined for the bundle",
"tsc": "Runs the Typescript compiler and produces the library version of the bundle"
}
Description
Include the
torchmodule, a PyTorch-like machine learning library designed for Source Academy.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Checklist: