-
Couldn't load subscription status.
- Fork 2
feat: get default versions #28
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
Conversation
WalkthroughThis update increments the package version from 0.2.9 to 0.2.10. It introduces three default constants and adds a static method Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MoveBuilder
Client->>+MoveBuilder: getDefaultVersions()
MoveBuilder-->>-Client: { bytecodeVersion: 6, compilerVersion: '2', languageVersion: '2' }
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/builder.ts (1)
358-358: Error message should refer to script bytes, not module bytes.The error message in
decodeScriptBytesmethod refers to "module bytes" which is inconsistent with the function's purpose.- throw new Error('Failed to decode module bytes') + throw new Error('Failed to decode script bytes')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
package.json(1 hunks)src/builder.ts(3 hunks)test/build.spec.ts(1 hunks)test/script.spec.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test & Build
🔇 Additional comments (4)
test/build.spec.ts (1)
26-32: Excellent test for the new functionality!The test ensures that the default values returned by
MoveBuilder.getDefaultValues()match the expected constants. This is a good addition that verifies the new functionality works as expected.src/builder.ts (3)
21-23: Well-defined constants for default versions.Creating constants for default version values improves maintainability by centralizing these values. This makes it easier to update them in the future if needed.
38-50: Great addition of the static method for accessing default values.The
getDefaultValuesmethod provides a clean interface for accessing the default version values. This is useful for clients that need to know these defaults without instantiating a MoveBuilder.
81-86: Fixed build configuration to use proper default values.The code now uses the defined constants instead of arbitrary values (0 or '0'). This ensures consistency across the codebase when default values are needed.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/builder.ts (1)
21-23: Consider adding documentation about version significanceThe constants define important default version values, but there's no explanation about why these specific values were chosen or what they represent (e.g., are they minimum supported versions, latest stable versions, etc.).
Consider adding a comment explaining the significance of these version numbers to help future developers understand when these might need to be updated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (2)
src/builder.ts(3 hunks)test/build.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/build.spec.ts
🔇 Additional comments (2)
src/builder.ts (2)
38-50: Good addition of a static accessor for default valuesCreating a static method to provide access to default configuration values is a good pattern. This makes the default values easily accessible to consumers of the library and centralizes them in one place.
81-86: Good refactoring to use constant defaultsReplacing hardcoded values with named constants improves code maintainability and ensures consistency. This change makes it easier to update version defaults in the future since they're defined in a single location.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/builder.ts (1)
38-50: Good addition of a static helper method, with minor suggestions.The static method provides a clean API to access default versions. Two minor improvements to consider:
- There's a typo in the JSDoc: "Movebuiler" should be "MoveBuilder"
- Consider adding an explicit return type for better API documentation
/** - * Get default value of a Movebuiler + * Get default value of a MoveBuilder * - version of bytecode * - version of move compiler * - version of move language version */ - static getDefaultVersions() { + static getDefaultVersions(): { + bytecodeVersion: number; + compilerVersion: string; + languageVersion: string + } { return { bytecodeVersion: DEFAULT_BYTECODE_VERSION, compilerVersion: DEFAULT_COMPILER_VERSION, languageVersion: DEFAULT_LANGUAGE_VERSION, } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (2)
src/builder.ts(3 hunks)test/build.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/build.spec.ts
🔇 Additional comments (2)
src/builder.ts (2)
21-23: Well-defined constants for default versions.These constants provide clear, maintainable default values that can be referenced throughout the codebase. Good practice to define these as constants rather than hardcoding values in multiple places.
81-86: Good use of default constants in configuration.The changes effectively use the newly defined constants as fallback values when options aren't provided. This replaces the previous hardcoded values with named constants, improving maintainability and making the default behavior more explicit.
Summary by CodeRabbit
MoveBuilder.