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

build: Use Mill wrapper and update to Mill 0.12.1 #900

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

nightscape
Copy link
Owner

@nightscape nightscape commented Nov 1, 2024

PR Type

enhancement, configuration changes


Description

  • Updated the CI workflow to use a Mill wrapper script, removing the previous setup for Mill version 0.11.8.
  • Changed the Mill version in the .mill-version file from 0.11.10 to 0.12.1.
  • Introduced a new Mill wrapper script that automates the download and setup of Mill, allowing version specification via command line or .mill-version file.

Changes walkthrough 📝

Relevant files
Configuration changes
ci.yml
Update CI workflow to use Mill wrapper                                     

.github/workflows/ci.yml

  • Removed the setup for Mill version 0.11.8.
  • Updated the test and publish commands to use the Mill wrapper script.
  • +3/-8     
    .mill-version
    Update Mill version to 0.12.1                                                       

    .mill-version

    • Updated Mill version from 0.11.10 to 0.12.1.
    +1/-1     
    Enhancement
    mill
    Add Mill wrapper script for automatic version management 

    mill

  • Added a new Mill wrapper script.
  • The script automatically downloads Mill from GitHub releases.
  • Supports specifying Mill version via command line or .mill-version
    file.
  • +241/-0 

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    github-actions bot commented Nov 1, 2024

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Command Consistency
    The PR introduces a new way to run Mill commands using a wrapper script. It's crucial to ensure that all Mill commands throughout the CI configurations are updated to use the wrapper script for consistency.

    Error Handling
    The script lacks robust error handling for network failures and other runtime issues during the download and setup of Mill. This could lead to build failures that are hard to diagnose.

    Copy link

    github-actions bot commented Nov 1, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Ensure the availability of 'curl' to prevent runtime errors in the script

    Consider adding a check to ensure that the 'curl' command is available before
    attempting to use it, to prevent script failure.

    mill [212]

    +command -v curl >/dev/null 2>&1 || { echo >&2 "curl is required but it's not installed.  Aborting."; exit 1; }
     ${CURL_CMD} -f -L -o "${DOWNLOAD_FILE}" "${DOWNLOAD_URL}"
    Suggestion importance[1-10]: 9

    Why: Adding a check for the availability of 'curl' before its usage is a crucial enhancement that prevents script failure due to missing dependencies. This is an important improvement for ensuring the script's reliability.

    9
    Best practice
    Use POSIX-compliant command options for better script portability and compatibility

    Replace the deprecated 'head --bytes=2' with 'head -c 2' for better compatibility
    and to adhere to POSIX standards.

    mill [117]

    -SYSTEM_MILL_FIRST_TWO_BYTES=$(head --bytes=2 "${MILL_IN_PATH}")
    +SYSTEM_MILL_FIRST_TWO_BYTES=$(head -c 2 "${MILL_IN_PATH}")
    Suggestion importance[1-10]: 8

    Why: Replacing 'head --bytes=2' with 'head -c 2' improves compatibility with POSIX standards, enhancing the portability of the script across different systems. This is a valuable change for maintaining cross-platform compatibility.

    8
    Enhancement
    Implement error handling for the publish command to enhance robustness

    Add error handling for the 'run' command in the publish job to manage potential
    failures gracefully.

    .github/workflows/ci.yml [146]

    -run: ./mill -i io.kipp.mill.ci.release.ReleaseModule/publishAll
    +run: ./mill -i io.kipp.mill.ci.release.ReleaseModule/publishAll || echo "Publish failed"
    Suggestion importance[1-10]: 7

    Why: Adding error handling to the publish command enhances the robustness of the CI workflow by providing a fallback message in case of failure. This is a useful improvement for debugging and monitoring purposes.

    7

    @nightscape nightscape force-pushed the mill-wrapper-and-update branch from 91e21bf to 6479134 Compare November 1, 2024 21:15
    @nightscape nightscape force-pushed the mill-wrapper-and-update branch from 6479134 to 439a6f5 Compare November 1, 2024 21:47
    @nightscape nightscape merged commit 6a41cc0 into main Nov 1, 2024
    40 checks passed
    @nightscape nightscape deleted the mill-wrapper-and-update branch November 1, 2024 21:57
    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.

    1 participant