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

fix missing src path when reporting pkg can't import #434

Merged
merged 1 commit into from
Oct 31, 2024
Merged

Conversation

Young-Flash
Copy link
Collaborator

No description provided.

Copy link

Suggestions from git diff Output for module.rs

  1. Potential Redundant Path Joining:

    self.source_dir
        .join(self.source.as_ref().unwrap_or(&String::new()))
        .join(pkg.rel.fs_full_name())
        .join(MOON_PKG_JSON)
        .display(),

    This snippet appears twice in the diff, suggesting a potential opportunity to refactor for code readability and maintainability. You might consider extracting this common path joining logic into a separate method to avoid repetition.

  2. Error Message Clarity:
    The error messages could be more user-friendly by including more context or suggestions for resolving the issue. For example:

    "{}: cannot import internal package `{}` in `{}`. Ensure the package is correctly referenced and exists.",

    This provides a clearer indication of what might be wrong and how to potentially fix it.

  3. Potential Null Pointer Dereference:
    The use of unwrap_or(&String::new()) suggests that there might be a scenario where self.source could be None. While unwrap_or provides a default, it might be safer and more idiomatic to use unwrap_or_else with a clearer fallback strategy or to handle the None case more explicitly to avoid potential runtime errors.

    self.source_dir
        .join(self.source.as_ref().unwrap_or_else(|| &String::new()))
        .join(pkg.rel.fs_full_name())
        .join(MOON_PKG_JSON)
        .display(),

These suggestions aim to improve code readability, maintainability, and safety, ensuring that the codebase remains robust and easy to understand.

@Young-Flash Young-Flash merged commit 1465662 into main Oct 31, 2024
14 checks passed
@Young-Flash Young-Flash deleted the fix_src branch October 31, 2024 02:52
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.

1 participant