fix: make FlowMigration project clients pass phpstan against manage-api ^10 - #119
Open
jirkasemmler wants to merge 1 commit into
Open
fix: make FlowMigration project clients pass phpstan against manage-api ^10#119jirkasemmler wants to merge 1 commit into
jirkasemmler wants to merge 1 commit into
Conversation
…pi ^10 CI on main has been red since c4ef6b4. #111 bumped the manage-api client to ^10, where getProject() and createProjectStorageToken() take int rather than string, and #113 added FlowMigration passing the project id through as a string. Both were green on their own; together they are not. Cast at the manage-api boundary rather than retyping the pipeline: projectId is a string all the way from the CLI input through BatchRunner, ProjectResult and the test fakes, and threading int through all of that is a bigger change than this bug warrants. The two remaining errors were $tokenInfo['token'] being mixed, since the token info is array<mixed>. Pulled into a local with an is_string() assert, the same way the other commands in this repo narrow API responses, which also drops the duplicate array access. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Changes:
FlowMigration/ProjectClientsFactory: cast$projectIdtointforgetProject()andcreateProjectStorageToken(), and narrow$tokenInfo['token']to a string with anis_string()assert.phpstan goes from 4 errors to 0. phpcs and phpunit (76 tests) stay green.
Why main is red. #111 bumped the manage-api client to
^10, where those two methods takeintinstead ofstring. #113 added FlowMigration, which passes the project id through as astring. Each PR was green on its own; the combination is not, so nothing flagged it until it was already onmain(c4ef6b4).Why cast instead of retyping.
projectIdis a string the whole way from CLI input throughBatchRunner,ProjectResultand the test fakes. Threadingintthrough all of it is a much larger change than this bug justifies, so the conversion happens at the manage-api boundary and the pipeline is untouched.Why not the baseline.
phpstan-baseline.neonalready carries 20+ entries for exactly these two messages, so adding four more would have matched precedent. Four real fixes seemed better than four more suppressions, but say the word and I will swap it for baseline entries instead.Worth merging before #115, #116, #117 and #118 - all four currently show red CI for this reason and nothing of their own.
Additional notes
🤖 Generated with Claude Code