Fix various code styles and CI issues#66
Conversation
📝 WalkthroughWalkthroughThis pull request updates documentation, configuration, and source code for a MediaWiki extension. The minimum required MediaWiki version is increased from 1.35 to 1.39, and the extension version is bumped to 3.0.0, with corresponding release notes in the README and extension files. Dependency versions in the Composer configuration are upgraded, and a new plugin is allowed. Additionally, the PHPStan configuration now loads a bootstrap file. Minor formatting adjustments are made in several source files, and the type declaration in a file fetcher, as well as control flow in a URL normalizer, are refined. Unit tests have been simplified accordingly. Changes
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (2)
103-103: Updated MediaWiki requirement in documentation.Correctly updated the minimum MediaWiki version to 1.39 to match the change in extension.json.
For consistency with other list items in this document, consider using a dash instead of an asterisk:
-* [MediaWiki] 1.39 or later (tested up to 1.43 and master) +- [MediaWiki] 1.39 or later (tested up to 1.43 and master)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
103-103: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
247-249: Added appropriate release notes for version 3.0.0.The release notes correctly document the MediaWiki requirement change.
The release date is set to 2025-02-25, which is in the future. Is this intentional or should it be updated to the actual release date?
For consistency with other list items in this document, consider using a dash instead of an asterisk in line 249:
-* Bump minimum MediaWiki requirements to 1.39 +- Bump minimum MediaWiki requirements to 1.39🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
249-249: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
README.md(2 hunks)composer.json(1 hunks)extension.json(2 hunks)phpstan.neon(1 hunks)src/Adapters/FileFetcher/MediaWikiFileFetcher.php(1 hunks)src/Domain/ContentRenderer/MarkdownRenderer.php(1 hunks)src/Domain/UrlNormalizer/BitbucketUrlNormalizer.php(2 hunks)src/Domain/UrlNormalizer/GitHubUrlNormalizer.php(1 hunks)src/EntryPoints/MediaWikiHooks.php(1 hunks)src/UseCases/Embed/EmbedUseCase.php(2 hunks)tests/Unit/Adapters/MediaWikiFileFetcherTest.php(4 hunks)
✅ Files skipped from review due to trivial changes (5)
- tests/Unit/Adapters/MediaWikiFileFetcherTest.php
- src/Domain/ContentRenderer/MarkdownRenderer.php
- src/EntryPoints/MediaWikiHooks.php
- src/Domain/UrlNormalizer/BitbucketUrlNormalizer.php
- src/UseCases/Embed/EmbedUseCase.php
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
103-103: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
249-249: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
🔇 Additional comments (8)
phpstan.neon (1)
10-11: Appropriate PHPStan bootstrap configuration added.Adding the MediaWiki AutoLoader.php as a bootstrap file is a good practice for PHPStan analysis of extensions, especially when upgrading to PHPStan 2.0.1 as mentioned in the PR summary.
src/Domain/UrlNormalizer/GitHubUrlNormalizer.php (1)
52-52: Good improvement to control flow logic.Converting to an if-elseif structure makes the relationship between these conditions clearer and ensures the second condition is only checked when the first is false, which is the intended behavior.
extension.json (2)
3-3: Appropriate version bump to 3.0.0.The version increment follows semantic versioning principles, correctly indicating a major version change with breaking changes.
16-16: Appropriate MediaWiki requirement update.Increasing the minimum MediaWiki version requirement to 1.39.0 aligns with the PR objectives and justifies the major version bump.
src/Adapters/FileFetcher/MediaWikiFileFetcher.php (1)
16-16: Improved type declaration syntaxThis change properly uses the modern PHP nullable type syntax (
?DomainCredentials) which better expresses that this parameter explicitly accepts null values. This is more consistent with static analysis expectations and improves code clarity.composer.json (3)
41-41: Updated MediaWiki CodeSnifferThe upgrade from version 37.0.0 to 45.0.0 aligns with addressing the code style issues mentioned in the PR description. This is a significant version jump that likely includes new rules and checks.
45-46: Added CodeSniffer Composer installer pluginAdding the
dealerdirect/phpcodesniffer-composer-installerplugin will automatically install and configure PHP_CodeSniffer standards, making it easier to maintain consistent code style throughout the project.
40-40:Details
❓ Verification inconclusive
Significant PHPStan upgrade
Upgrading from version 0.12.99 to 2.0.1 is a major change that could introduce breaking changes in static analysis. Ensure that the new bootstrap file mentioned in the PR description properly handles any compatibility issues.
🏁 Script executed:
#!/bin/bash # Check if there are any PHPStan errors after the upgrade if [ -f "phpstan.neon" ]; then echo "PHPStan configuration file found, examining bootstrap configuration:" cat phpstan.neon | grep -A 5 "bootstrap" fi # Check PHPStan version compatibility implications echo "Checking for major PHPStan version changes:" if [ -f "vendor/bin/phpstan" ]; then vendor/bin/phpstan --version fiLength of output: 393
Attention: PHPStan Upgrade Verification
The upgrade to PHPStan 2.0.1 from 0.12.99 is indeed a major change. The configuration in
phpstan.neonshows that the bootstrap file (../../includes/AutoLoader.php) is still in use, which is good. However, the shell script did not find thevendor/bin/phpstanexecutable—which suggests that PHPStan may not be installed or available yet. Please ensure that:
- The bootstrap file (
../../includes/AutoLoader.php) in yourphpstan.neoncorrectly handles compatibility requirements for PHPStan 2.0.1.- The PHPStan binary is present (e.g., after running
composer install) and confirms the correct version.- A full run of PHPStan does not introduce new static analysis errors due to the upgrade.
|
In general, I think it's better to leave version number changes to a separate PR. Although we're not planning any new changes now, so not a problem here. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
103-103: MediaWiki Version Requirement Update & List Marker Consistency
The update to "* [MediaWiki] 1.39 or later (tested up to 1.43 and master)" correctly reflects the new minimum MediaWiki requirements per the PR objectives. However, according to the markdownlint recommendation (MD004), it is preferred to use a dash (-) rather than an asterisk for unordered list items. Consider updating the list marker to maintain consistency with the project's style guidelines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
103-103: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)src/Domain/UrlNormalizer/BitbucketUrlNormalizer.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Domain/UrlNormalizer/BitbucketUrlNormalizer.php
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
103-103: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
249-249: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
🔇 Additional comments (1)
README.md (1)
247-250: Release Note Wording & List Marker Format
The addition of the release note for version 3.0.0 correctly documents the update. To maintain consistency with previous release notes and the project's guidelines, consider writing the release notes in past tense (e.g., "Bumped minimum MediaWiki requirements to 1.39"). Additionally, similar to the MediaWiki requirement bullet, updating the unordered list marker from an asterisk to a dash would address the markdownlint warning.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
249-249: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
3 participants
Follow up to #65
^0.12.99to^2.0.2^4.10to^4.30.037.0.0to45.0.0dealerdirect/phpcodesniffer-composer-installercomposer pluginSummary by CodeRabbit
Summary by CodeRabbit
vimeo/psalm,phpstan/phpstan, andmediawiki/mediawiki-codesniffer.