Skip to content

chore: Simplify yarn and alpine version update #2258

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

Merged
merged 1 commit into from
Jul 16, 2025

Conversation

orgads
Copy link
Contributor

@orgads orgads commented Jul 10, 2025

Description

Before commit 3f6cb08 (fix: only update yarn if there are other changes to the dockerfile), yarnVersion depended on SKIP, so the substitute that followed made sense. This commit removed the condition and changed it to just substitute the version with itself, which doesn't make much sense.

Just remove it, and also move the real replace inside SKIP condition.

Motivation and Context

Testing Details

Tested yarn update.

Example Output(if appropriate)

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.

@orgads orgads changed the title chore: Simplify yarn version update chore: Simplify yarn and alpine version update Jul 10, 2025
@PeterDaveHello PeterDaveHello requested a review from Copilot July 10, 2025 19:57
Copy link

@Copilot 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 streamlines the Dockerfile update script by removing redundant initial fetches of Alpine and Yarn versions and relocating the Yarn version lookup so it's only performed when updates are desired.

  • Eliminated the early assignment of alpine_version and initial Yarn version fetch
  • Removed rewriting the Dockerfile with its own current Yarn version
  • Moved the real Yarn version fetch and substitution inside the SKIP check
Comments suppressed due to low confidence (1)

@orgads orgads force-pushed the yarn-update branch 2 times, most recently from 9c3e1c2 to c723fcb Compare July 16, 2025 11:42
Before commit 3f6cb08 (fix: only update yarn if there are other changes
to the dockerfile), yarnVersion depended on SKIP, so the substitute that
followed made sense. This commit removed the condition and changed it to
just substitute the version with itself, which doesn't make much sense.

Just remove it, and also move the real replace inside SKIP condition.

Also remove initial assignment to alpine_version, which is never used.
@orgads orgads requested a review from SimenB July 16, 2025 11:43
@SimenB SimenB merged commit dd95409 into nodejs:main Jul 16, 2025
3 checks passed
@orgads orgads deleted the yarn-update branch July 16, 2025 15:32
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.

2 participants