Repository navigation
Add php-cs-fixer, make the facade lint real, and harden CI - #107
Merged
alies-dev merged 11 commits intoOct 23, 2026
Merged
Conversation
The services comment in config/swap.php told users to pass access_key to apilayer_fixer and apilayer_exchange_rates_data, which Exchanger rejects (they take api_key), showed 'api_key' => 'access_key' for currency_converter, and listed less than half of the registered services. List every service Exchanger registers with the options its processOptions() reads, and add the missing enterprise flags and the National Bank of Denmark to the docs.
Swap 4.6 pulls in Exchanger 2.11 with the provider fixes the config reference now documents. The 2.x-dev alias matches the branch policy of the sister packages.
Illuminate\Contracts\Cache\Repository extends Psr\SimpleCache\CacheInterface since Laravel 5.8, so on every Laravel version this package can install on (PHP 8.2+) the php-cache bridge fallback is unreachable. Drop it with its Laravel 5.7 docs, and test that the store named in swap.cache receives the cached rates.
Merge the package config in register() as the Laravel package docs recommend, publish it to the application's configPath() instead of a hand built base path, and register Swap\Swap as a container alias of the swap service rather than a second binding. The provider only registers the swap singleton, so implement DeferrableProvider and list Swap\Swap in provides(): it is now loaded when Swap is first resolved instead of on every request. Artisan's console kernel loads all deferred providers before running a command, so vendor:publish still sees the config path registered in boot(). Cover publishing, the alias, deferral, the custom HTTP client and request factory, services disabled with false and service order.
The provider and facade extend illuminate/support and use illuminate/contracts, but composer.json required neither, so nothing stopped installs on Laravel versions that were never tested. Require them for Laravel 12 and 13, the majors the test bench and CI cover. Laravel 11 is left out: every 11.x release has open security advisories, so Composer refuses to install it by default. Laravel 10 needs orchestra/testbench 8, which only runs on PHPUnit 10. CI now runs the tests for each PHP and Laravel pair; Laravel 13 needs PHP 8.3. Lumen, whose last release is built on Laravel 11, is dropped from the keywords.
The README promised Laravel 5.5+ and Lumen support. Name the Laravel versions CI tests instead, and keep the Lumen setup section with a note that it is no longer tested and only applies to earlier releases.
The composer cs:fix and cs:check scripts pointed at a php-cs-fixer binary that was never required. Require it and reuse Swap's config so the sister packages share one code style.
facade.php only checks with --lint; without it, it rewrites the docblock and exits 0, so the CI step always passed. The composer facade:fix and facade:lint scripts also lost their backslashes to the shell (ReflectionException for "SwapLaravelFacadesSwap") and passed a --write flag the tool does not know. Quote the class name, pass --lint to facade:lint and run that script in CI. Drop the @api tag that the documenter strips; Psalm's UnusedClass for the facade is suppressed in psalm.xml instead.
Give every workflow read-only contents permission, a timeout, and a concurrency group that cancels superseded pull request runs, and install dependencies with ramsey/composer-install for caching. Replace the Code Style workflow with Coding Standards, which runs composer cs:check next to the facade docblock lint and tells contributors which script fixes a failure.
Match Swap's PHPUnit settings so a warning or a PHPUnit deprecation fails CI instead of scrolling by, and print details for warnings, deprecations and skipped tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
composer cs:fixandcs:checkcalledvendor/bin/php-cs-fixer, which was never required, so code style was not checked anywhere. The "Code Style" workflow only ran the facade documenter, and that step could not fail:facade.phpcompares docblocks only with--lint; without it, it rewrites the facade in the CI checkout and exits 0. Thefacade:lintandfacade:fixComposer scripts were broken too, because the shell strips the unquoted backslashes:The workflows also ran with the default token permissions, no timeout, and no cancellation of superseded runs.
Solution
friendsofphp/php-cs-fixer^3.95 with Swap's.php-cs-fixer.dist.php(PER-CS 3.0 and PHP 8.2 migration rules). The result ofcomposer cs:fixis a separate commit (three concatenation spaces).--linttofacade:lint. A real lint showed that the documenter drops the@apitag added for Psalm's unused code check, so the tag is removed andUnusedClassis suppressed for the facade inpsalm.xmlwith the reason.composer cs:checkandcomposer facade:lint, each with a hint naming the fixing script.permissions: contents: read, atimeout-minutes, a concurrency group that cancels superseded pull request runs, andramsey/composer-install@v3withdependency-versions: highest(the Laravel matrix pins its major withcomposer require --no-updatefirst, so this still resolves the newest release of that major).phpunit.xml.distnow fails on warnings, PHPUnit deprecations and empty suites and prints their details, like Swap.laravel/facade-documenterstays ondev-mainfrom its VCS repository: it has no tags and is not published on Packagist.minimum-stability: betaalso stays, becausepsalm/plugin-laravel4 requires Psalm 7, which is still a release candidate.