[command] show installed DevTools version in list output#340
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughDevTools now accepts a VersionChecker, resolves the installed DevTools version at bootstrap via getCurrentVersion(), and passes it into the Symfony Application so ChangesVersion Metadata Integration
Sequence DiagramsequenceDiagram
participant DevTools
participant VersionCheckerInterface
participant ComposerVersionChecker
participant InstalledVersions
participant SymfonyApplication
DevTools->>VersionCheckerInterface: getCurrentVersion()
VersionCheckerInterface->>ComposerVersionChecker: getCurrentVersion()
ComposerVersionChecker->>InstalledVersions: isInstalled('fast-forward/dev-tools')
ComposerVersionChecker->>InstalledVersions: getPrettyVersion() / getVersion()
InstalledVersions-->>ComposerVersionChecker: prettyVersion | version | error
ComposerVersionChecker-->>VersionCheckerInterface: resolvedVersion (or VERSION_UNKNOWN)
VersionCheckerInterface-->>DevTools: resolvedVersion
DevTools->>SymfonyApplication: parent::__construct(name, resolvedVersion)
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🚀 Preview is available for this pull request. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Console/DevTools.php`:
- Around line 99-103: Update the PHPDoc for the method in class DevTools that
"resolves the running DevTools version for command metadata" to use RFC 2119
wording: state that the method MUST return the current package version when
available and MUST fall back to a safe string otherwise, include the `@return`
string tag, and optionally add that callers SHOULD handle the fallback value;
ensure the docblock uses uppercase RFC2119 keywords (MUST, SHOULD) and
references the method behavior.
- Around line 106-108: Guard Composer metadata access by first calling
InstalledVersions::isInstalled(self::PACKAGE) and only then call
InstalledVersions::getPrettyVersion(self::PACKAGE) /
InstalledVersions::getVersion(self::PACKAGE); additionally wrap those calls in a
try-catch for \OutOfBoundsException (or generic \Throwable) so any thrown
exception falls back to self::VERSION_UNKNOWN; update the return logic in the
method that currently calls InstalledVersions::getPrettyVersion / getVersion to
use the isInstalled check and the try-catch so the null-coalescing fallback is
reachable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 40a37296-700f-4e2c-9845-0a428f7f504b
📒 Files selected for processing (4)
.github/wikiCHANGELOG.mdsrc/Console/DevTools.phptests/Console/DevToolsTest.php
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38abd3edee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/Console/DevTools.php (1)
99-107:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRFC 2119 keywords still missing from the method docblock.
The past review comment correctly identified that this docblock lacks RFC 2119 keywords required by coding standards. The method MUST return the current package version when available and MUST fall back to
VERSION_UNKNOWNotherwise.📝 Suggested enhancement
/** - * Resolves the running DevTools version for command metadata. + * Resolves the running DevTools version for application metadata display. + * + * The method MUST return the current package version from the version checker when available. + * The method MUST fall back to VERSION_UNKNOWN when version resolution returns null. * - * `@return` string the current package version or a safe fallback + * `@return` string the version string for Symfony Console Application metadata */As per coding guidelines,
**/*.php: "Use RFC 2119 keywords (MUST, SHOULD, MAY, etc.) in PHPDoc comments".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Console/DevTools.php` around lines 99 - 107, Update the docblock for resolveVersion() to include RFC 2119 wording: state that resolveVersion() MUST return the current package version (obtained via $this->versionChecker->resolveCurrentVersion()) when available and MUST fall back to self::VERSION_UNKNOWN when resolveCurrentVersion() returns null; mention the return type remains string and that resolveCurrentVersion() may return null which triggers the fallback to VERSION_UNKNOWN.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/SelfUpdate/ComposerVersionChecker.php`:
- Around line 70-72: The PHPDoc for the method that "Returns the installed
DevTools version without running external Composer commands" (method:
ComposerVersionChecker::getInstalledVersion) is missing RFC 2119 terms; update
the docblock to state the contract using MUST/MUST NOT: explicitly document that
ComposerVersionChecker::getInstalledVersion MUST return the installed version
string when metadata is available and MUST return null when resolution fails,
and include any relevant MAY/SHOULD notes if needed (e.g., about caching or
exceptions) so the method's behavior is unambiguous.
- Around line 70-77: The resolveCurrentVersion() implementation must catch cases
where InstalledVersions::getPrettyVersion() or ::getVersion() throw
OutOfBoundsException and return null per the ?string contract; wrap the calls to
InstalledVersions in a try/catch that catches OutOfBoundsException (or
Throwable), return the pretty version if available, fallback to getVersion(),
and return null if an exception occurs or neither value is present (referencing
resolveCurrentVersion and InstalledVersions).
In `@src/SelfUpdate/VersionCheckerInterface.php`:
- Around line 32-37: Update the PHPDoc for the resolveCurrentVersion() method to
include RFC 2119 keywords: state that implementations MUST return the currently
installed DevTools version string when it is available and MAY return null if
the version cannot be resolved; keep the existing short description and `@return`
type but add these MUST/MAY sentences to the docblock for
resolveCurrentVersion().
---
Duplicate comments:
In `@src/Console/DevTools.php`:
- Around line 99-107: Update the docblock for resolveVersion() to include RFC
2119 wording: state that resolveVersion() MUST return the current package
version (obtained via $this->versionChecker->resolveCurrentVersion()) when
available and MUST fall back to self::VERSION_UNKNOWN when
resolveCurrentVersion() returns null; mention the return type remains string and
that resolveCurrentVersion() may return null which triggers the fallback to
VERSION_UNKNOWN.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1d03fcda-1c09-4122-ac26-6f243b4197db
📒 Files selected for processing (5)
.github/wikisrc/Console/DevTools.phpsrc/SelfUpdate/ComposerVersionChecker.phpsrc/SelfUpdate/VersionCheckerInterface.phptests/Console/DevToolsTest.php
Summary
Show the installed DevTools version in the console application metadata so
listand--versionreflect the running release, improving runtime visibility for troubleshooting and reproducibility.Changes
fast-forward/dev-toolsversion resolution tosrc/Console/DevTools.phpby passing a secondApplicationconstructor argument.src/Console/DevTools::resolveVersion()with a safe fallback for environments without installed version metadata.tests/Console/DevToolsTest.phpto confirm application version metadata is populated.CHANGELOG.mdentry underUnreleaseddescribing the command-list version visibility improvement.Testing
vendor/bin/phpunit tests/Console/DevToolsTest.php(not yet run)git committriggeredcomposer dev-tools, which currently fails later in coverage reporting with an existingRuntimeEnvironmentTestrisky test unrelated to this change.Changelog
CHANGELOG.mdentry.Closes #339