Skip to content

Conversation

@PeterDaveHello
Copy link
Member

Description

Clarify update.sh status messages by comparing each generated Dockerfile with the existing file before replacing it. Move the Yarn version update ahead of the comparison and emit either “updated” when the content changes or “already up to date.” when it does not.

GitHub Copilot Summary

This pull request refactors the logic for updating Dockerfiles in the update_node_version() function in update.sh. The main change is a switch from using diff to cmp for file comparison, and updating the messaging and cleanup flow for temporary files.

Improvements to update logic:

  • Replaced the use of diff -q with cmp -s to compare ${dockerfile}-tmp and ${dockerfile}, simplifying the check for file changes.
  • Updated messaging to use the info function for consistent output when a Dockerfile is already up to date or has been updated.
  • Improved cleanup by removing the temporary file ${dockerfile}-tmp if no changes are detected, and only moving it to ${dockerfile} when updates are made.

Motivation and Context

Previous runs could print “updated!” even when the final Dockerfile was unchanged, leading to confusing output after git status showed no diffs. This change makes the script’s messaging align with the actual state of the files.

Testing Details

  • ./update.sh 20 bookworm
  • ./update.sh 22

Example Output(if appropriate)

Updating version 20...
20/bookworm/Dockerfile already up to date.
Done!

Types of changes

  • Documentation
  • Version change (Update, remove or add more Node.js versions)
  • Variant change (Update, remove or add more variants, or versions of variants)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Other (none of the above)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING.md document.
  • All new and existing tests passed.

Compare each generated Dockerfile with its existing copy before
replacing it, so the script only announces an update when the content
actually changes. Otherwise keep the original file and print an
'already up to date' message to eliminate false positives.
@PeterDaveHello PeterDaveHello requested review from a team and Copilot November 6, 2025 19:45
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the Dockerfile update logic in update.sh to improve efficiency and consistency. The main goal is to avoid unnecessary file operations and use consistent output messaging.

  • Moved the YARN_VERSION sed operation to always execute before comparison (when SKIP is false)
  • Changed file comparison from diff -q to cmp -s and added conditional handling to avoid overwriting unchanged files
  • Standardized output messages to use the info function instead of echo

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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