Skip to content

Bumping js-slang to 1.0.92#684

Closed
martin-henz wants to merge 6 commits intomasterfrom
1092
Closed

Bumping js-slang to 1.0.92#684
martin-henz wants to merge 6 commits intomasterfrom
1092

Conversation

@martin-henz
Copy link
Copy Markdown
Member

@martin-henz martin-henz commented Apr 24, 2026

Upgrading to 1.0.92 with the help of Claude Code. Several rounds of correction, to pass CICD and get compliance with gemini-code reviews.

@martin-henz martin-henz marked this pull request as draft April 24, 2026 04:16
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request upgrades js-slang to version 1.0.92 and introduces several type-related updates and refactorings across multiple bundles. The reviewer recommends reverting changes in the binary_tree bundle that replaced list, head, and tail abstractions with direct array literals and indexing, as these leak internal implementation details and reduce maintainability.

Comment thread src/bundles/binary_tree/src/functions.ts Outdated
Comment thread src/bundles/binary_tree/src/functions.ts Outdated
export function left_branch(t: BinaryTree): BinaryTree {
throwIfNotNonEmptyTree(t, left_branch.name);
return head(tail(t));
return t[1][0];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Directly accessing array indices (t[1][0]) leaks the internal implementation details of how lists are represented in js-slang. This makes the code more brittle and harder to maintain. It is recommended to use the head and tail abstractions.

Suggested change
return t[1][0];
return head(tail(t));

Comment thread src/bundles/binary_tree/src/functions.ts Outdated
@martin-henz martin-henz marked this pull request as ready for review April 24, 2026 05:26
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