Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| name: Build runner | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v6 | ||
|
|
||
| - name: Enable corepack | ||
| run: corepack enable | ||
|
|
||
| - name: Setup node | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 22 | ||
| cache: yarn | ||
|
|
||
| - name: Install dependencies | ||
| run: yarn install --frozen-lockfile | ||
|
|
||
| - name: Build | ||
| run: yarn run build --all || yarn build | ||
|
|
||
| - name: Upload artifacts | ||
| id: deployment | ||
| uses: actions/upload-pages-artifact@v3 | ||
| with: | ||
| path: dist/ | ||
|
|
||
| deploy: |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a minimal JavaEvaluator and associated conductor runner infrastructure to support executing Java class files encoded as base64, along with corresponding unit tests. Feedback on the changes includes: initializing the entries array in ExceptionTable to prevent runtime errors, importing EVALUATOR directly from the evaluator module to avoid using @ts-expect-error, and addressing the unused setupJVM call in JavaEvaluator which currently acts as dead code.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| insert(from: number, to: number, target: number, type: ClassData): void { | ||
| var entry = new Entry(from, to, target, type) | ||
| const entry = new Entry(from, to, target, type) |
There was a problem hiding this comment.
The this.entries array is declared but never initialized (e.g., to []), which will cause a runtime TypeError when push is called. Please initialize this.entries to an empty array. Note that the retrieve method also has an issue where returning from inside forEach does not return from the outer function; consider refactoring it to use find or a for...of loop.
if (!this.entries) {
this.entries = []
}
const entry = new Entry(from, to, target, type)| // @ts-expect-error — __EVALUATOR__ is replaced at build time by bundlers | ||
| import { __EVALUATOR__ } from './index' |
There was a problem hiding this comment.
Instead of importing __EVALUATOR__ from ./index (which does not export it) and using @ts-expect-error, you can import it directly from ./evaluator where it is declared and exported as default. This provides full type safety and avoids compilation/bundling workarounds.
| // @ts-expect-error — __EVALUATOR__ is replaced at build time by bundlers | |
| import { __EVALUATOR__ } from './index' | |
| import __EVALUATOR__ from './evaluator' |
| setupJVM({ | ||
| callbacks: { | ||
| readFileSync: (path: string) => { | ||
| throw new Error('readFileSync not available in conductor JavaEvaluator') | ||
| }, | ||
| stdout: (m: string) => this.conductor.sendOutput(m), | ||
| stderr: (m: string) => this.conductor.sendOutput(`ERR: ${m}`) | ||
| } | ||
| }) |
There was a problem hiding this comment.
setupJVM returns a runner function that must be executed to actually start the JVM. Currently, the returned function is discarded, meaning the JVM is never run. Since running the class via the in-memory runner is not yet implemented, this call is dead code and can be removed or commented out to avoid unnecessary instantiation overhead.
// setupJVM call removed as it is currently unused dead code
Coverage report
Show new covered files 🐣
Test suite run success1136 tests passing in 65 suites. Report generated by 🧪jest coverage report action from d637754 |
martin-henz
requested review from
AaravMalani
and removed request for
martin-henz
2 participants
No description provided.