-
-
Notifications
You must be signed in to change notification settings - Fork 34
Fix #279: Add link to upgrade notes to news text #324
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
base: master
Are you sure you want to change the base?
Conversation
samdark
commented
Jun 15, 2025
Q | A |
---|---|
Is bugfix? | ❌ |
New feature? | ✔️ |
Breaks BC? | ❌ |
Fixed issues | #279 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a conditional link to the repository’s UPGRADE.md in the release summary and ensures the output is trimmed before printing.
- Initialized and populated an
$upgradeNotesLink
whenUPGRADE.md
exists - Incorporated the link into the release message heredoc
- Trimmed the final message before passing it to
$io->info()
Comments suppressed due to low confidence (2)
src/App/Command/Release/MakeCommand.php:311
- Add tests to verify that the upgrade notes link is shown when
UPGRADE.md
exists and omitted when it does not, to prevent regressions.
if (file_exists($upgradePath)) {
src/App/Command/Release/MakeCommand.php:312
- To avoid potential issues with special characters in the package name or version, consider wrapping
$packageName
and$versionToRelease
in rawurlencode() when building the URL.
$upgradeNotesLink = "\n\nSee [UPGRADE.md](https://github.com/yiisoft/$packageName/blob/$versionToRelease/UPGRADE.md) for upgrade notes.";
@@ -306,13 +306,20 @@ private function displayReleaseSummary(Package $package, ComposerConfig $compose | |||
|
|||
$changes = implode("\n", $changes); | |||
|
|||
$upgradeNotesLink = ''; | |||
$upgradePath = $package->getPath() . '/UPGRADE.md'; | |||
if (file_exists($upgradePath)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider using is_file() instead of file_exists() to ensure the path points to a regular file rather than a directory.
if (file_exists($upgradePath)) { | |
if (is_file($upgradePath)) { |
Copilot uses AI. Check for mistakes.
@@ -306,13 +306,20 @@ private function displayReleaseSummary(Package $package, ComposerConfig $compose | |||
|
|||
$changes = implode("\n", $changes); | |||
|
|||
$upgradeNotesLink = ''; | |||
$upgradePath = $package->getPath() . '/UPGRADE.md'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs for major releases only.
$upgradeNotesLink = ''; | ||
$upgradePath = $package->getPath() . '/UPGRADE.md'; | ||
if (file_exists($upgradePath)) { | ||
$upgradeNotesLink = "\n\nSee [UPGRADE.md](https://github.com/yiisoft/$packageName/blob/$versionToRelease/UPGRADE.md) for upgrade notes."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good if we link to exact notes with anchor. For example:
$upgradeNotesLink = "\n\nSee [UPGRADE.md](https://github.com/yiisoft/$packageName/blob/$versionToRelease/UPGRADE.md) for upgrade notes."; | |
$upgradeNotesLink = "\n\nSee [UPGRADE.md](https://github.com/yiisoft/$packageName/blob/$versionToRelease/UPGRADE.md#1-0) for upgrade notes."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tricky since header in these notes is created manually and doesn't correspond to version being released.