Skip to content

fix: better check-updates logging#163

Closed
paolomainardi wants to merge 1 commit into
masterfrom
feature/sparkdock-better-check-updates
Closed

fix: better check-updates logging#163
paolomainardi wants to merge 1 commit into
masterfrom
feature/sparkdock-better-check-updates

Conversation

@paolomainardi

@paolomainardi paolomainardi commented Aug 23, 2025

Copy link
Copy Markdown
Member

PR Type

Bug fix


Description

  • Improved update checking logic in macOS script

  • Added validation for git log output

  • Enhanced logging reliability for available updates

  • Fixed potential empty output display issues


Changes walkthrough 📝

Relevant files
Bug fix
sparkdock.macos
Improve update checking output validation                               

bin/sparkdock.macos

  • Store git log output in DIFF variable before displaying
  • Add check to ensure DIFF is not empty before showing updates
  • Prevent display of empty update messages
  • +5/-2     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copilot AI review requested due to automatic review settings August 23, 2025 14:35

    Copilot AI left a comment

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

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

    Copilot wasn't able to review any files in this pull request.

    @sparkfabrik-ai-bot

    Copy link
    Copy Markdown

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Logic Issue

    The validation check for git log output may prevent legitimate updates from being displayed. If git log returns empty output (no commits between HEAD and origin branch), the function will still return 0 indicating updates are available, but won't display anything to the user, creating inconsistent behavior.

    DIFF=$(git --no-pager log --oneline HEAD..origin/${DEFAULT_BRANCH})
    if [ -n "$DIFF" ]; then
        echo "Updates available:"
        echo "$DIFF"
    fi

    @sparkfabrik-ai-bot

    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Remove redundant empty check

    The additional check for non-empty DIFF is redundant since the outer condition
    already ensures LOCAL != REMOTE, which guarantees commits exist. This adds
    unnecessary complexity without providing value.

    bin/sparkdock.macos [48-52]

    -DIFF=$(git --no-pager log --oneline HEAD..origin/${DEFAULT_BRANCH})
    -if [ -n "$DIFF" ]; then
    -    echo "Updates available:"
    -    echo "$DIFF"
    -fi
    +echo "Updates available:"
    +git --no-pager log --oneline HEAD..origin/${DEFAULT_BRANCH}
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion is technically correct that LOCAL != REMOTE implies commits exist, but the additional check for non-empty DIFF provides defensive programming and better error handling in edge cases where git commands might fail or return unexpected results.

    Low

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants