Skip to content
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

feat: support get cli args & env var #73

Merged
merged 6 commits into from
Nov 15, 2024
Merged

feat: support get cli args & env var #73

merged 6 commits into from
Nov 15, 2024

Conversation

Young-Flash
Copy link
Collaborator

No description provided.

Copy link

peter-jerry-ye-code-review bot commented Nov 12, 2024

From the provided git diff output, I've identified several issues that could potentially lead to bugs, typos, or inconsistencies in the MoonBit codebase. Here are the top three problems I've observed:

  1. Misuse of String Substring Method:

    • In multiple files (e.g., json5/lex_number.mbt, json5/util.mbt), the substring method is called with potentially incorrect arguments due to the use of the ~ operator for optional parameters. This operator should not be used in this context.
    • Problem: The ~ operator is used for optional parameters, but it seems incorrectly applied here, leading to potential issues with string manipulation.
    • Suggestion: Remove the ~ operator from the substring method calls to correctly pass the start and end indices.
  2. Incorrect Use of Optional Parameters in Function Calls:

    • In several files (e.g., time/duration.mbt, time/period.mbt), functions are called with optional parameters using the ~ operator, which is not standard for calling functions in MoonBit.
    • Problem: The ~ operator is used for optional parameters in function definitions, but it should not be used when calling those functions.
    • Suggestion: Remove the ~ operator from the function calls to correctly pass the optional parameters.
  3. Potential Logic Error in UUID String Conversion:

    • In uuid/uuid.mbt, the to_string function uses rv.to_string() which might be incorrect if rv is already a string buffer.
    • Problem: The to_string method is called on an array of bytes (rv), which might lead to incorrect string conversion.
    • Suggestion: Use rv.to_unchecked_string() instead of rv.to_string() to directly convert the byte array to a string without additional processing, assuming to_unchecked_string is the correct method for this context.

These issues, if not addressed, could lead to runtime errors, incorrect string manipulations, or unexpected behavior in the code. Addressing these suggestions will help ensure the code is more robust and follows the correct conventions for MoonBit.

@Young-Flash
Copy link
Collaborator Author

this requires moonbitlang/moon#470, ci would pass after next stable release

Copy link
Collaborator

@peter-jerry-ye peter-jerry-ye left a comment

Choose a reason for hiding this comment

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

LGTM

@mengzhuo
Copy link

Related #71

@peter-jerry-ye peter-jerry-ye linked an issue Nov 15, 2024 that may be closed by this pull request
@Young-Flash Young-Flash merged commit 56986f4 into main Nov 15, 2024
7 of 8 checks passed
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.

support get cli args
3 participants