Skip to content

fix(internal/librarian/php): remove php fallback generator installation#7038

Merged
sofisl merged 17 commits into
googleapis:mainfrom
sofisl:removePHPFallbackInstall
Jul 24, 2026
Merged

fix(internal/librarian/php): remove php fallback generator installation#7038
sofisl merged 17 commits into
googleapis:mainfrom
sofisl:removePHPFallbackInstall

Conversation

@sofisl

@sofisl sofisl commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

This PR removes the fallback hardocded generator installation for PHP in lieu of the new generator installation command.

For #6630

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request simplifies the PHP generator setup by removing the fallback installation and location logic, strictly requiring the PHP generator wrapper to be located in the bin directory. It also updates the installation process to enforce the presence of tools and Composer configurations, with corresponding test updates. The feedback highlights a potential issue in the test file where ignoring the error from binDir() could lead to unexpected behavior, suggesting that the error should be handled explicitly.

Comment thread internal/librarian/php/generate_test.go Outdated
@sofisl sofisl changed the title fix: remove php fallback generator installation fix(internal/librarian/php): remove php fallback generator installation Jul 24, 2026
@sofisl
sofisl marked this pull request as ready for review July 24, 2026 20:07
@sofisl
sofisl requested a review from a team as a code owner July 24, 2026 20:07
Comment thread .github/actions/setup-librarian/action.yaml Outdated
noahdietz
noahdietz previously approved these changes Jul 24, 2026
Comment thread .github/actions/setup-librarian/action.yaml Outdated
Comment thread internal/librarian/php/generate_test.go Outdated
@noahdietz
noahdietz dismissed their stale review July 24, 2026 20:23

Another reviewer commented with same comment, want to get alignment

Comment thread internal/librarian/php/install.go
sofisl and others added 2 commits July 24, 2026 14:56
Co-authored-by: Joe Wang <106995533+JoeWang1127@users.noreply.github.com>
Signed-off-by: sofisl <55454395+sofisl@users.noreply.github.com>
@JoeWang1127

Copy link
Copy Markdown
Contributor

Could you fix the linter check?

Comment thread internal/librarian/php/install.go
Comment thread .github/actions/setup-librarian/action.yaml Outdated
Comment thread internal/librarian/php/install.go
@sofisl
sofisl merged commit cf36be0 into googleapis:main Jul 24, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants