Skip to content

chore(AdsAdManager): remove orphaned files from previous proto refactors - #9519

Open
sofisl wants to merge 2 commits into
mainfrom
chore/remove-orphaned-ads-files
Open

chore(AdsAdManager): remove orphaned files from previous proto refactors#9519
sofisl wants to merge 2 commits into
mainfrom
chore/remove-orphaned-ads-files

Conversation

@sofisl

@sofisl sofisl commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

This PR removes legacy/orphaned PHP files in AdsAdManager that no longer correspond to upstream googleapis definitions.

Historically, when proto files or messages were renamed/moved in googleapis, the PHP generator (via OwlBot) successfully generated the new files but failed to delete the old ones, leaving thousands of lines of dead code in the repository. We discovered these while testing the new Librarian migration tool, which strictly cleans output directories and correctly flags these as deletions.

Evidence & Tracing:

CompanyCreditStatusEnum.php & CompanyTypeEnum.php

Origin: Generated from company_credit_status_enum.proto.
The Change: In googleapis commit 1bc55112f708b1082ebb541f08e0454f2c78699f, this proto was renamed to company_enums.proto (fix!: Moved Company enums to a separate file).
Result: The new enums were generated successfully, but the old PHP files were left behind.
Report/* files (e.g., DataTable.php, Visibility.php)

Origin: Generated from report_messages.proto.
The Change: In the same googleapis commit 1bc55112f708b1082ebb541f08e0454f2c78699f, Report messages were extracted and restructured (fix!: Moved Report messages to a separate file).
Result: The old src/V1/Report/ directory and its contents were orphaned.
Schedule.php and Schedule/* directory (e.g., Frequency.php)

Origin: Schedule used to be a top-level message in report_messages.proto.
The Change: In googleapis commit a15a5b7adf695d1b6b17359ff64bfb4a83d0750b, these messages were moved (fix!: Moved Schedule messages from report_messages.proto to report_delivery.proto). Furthermore, Schedule was nested inside a new ScheduleOptions message.
Result: The generator correctly created the new files under src/V1/ScheduleOptions/Schedule/ (which are currently active in main), but OwlBot never deleted the old src/V1/Schedule.php and src/V1/Schedule/ files.
Removing these 25 files cleans up ~9,000 lines of dead code and allows for accurate 0-diff testing against the new Librarian generator.\n\nSafety / Backwards Compatibility Note:\nAny customer code attempting to use these deleted classes is already broken and crashing in production. \nAttempting to instantiate them immediately throws a Fatal PHP Error.\n\nExample instantiation of an orphaned class:\nphp\n<?php\nrequire __DIR__ . '/vendor/autoload.php';\nuse Google\Cloud\AdsAdManager\V1\CompanyTypeEnum;\n\n$enum = new CompanyTypeEnum();\n\nOutput:\n\nPHP Fatal error: Uncaught Error: Class "Google\Cloud\AdsAdManager\V1\CompanyTypeEnum" not found\n\nDeleting these files simply cleans up the dead code and allows static analysis tools to correctly flag them as non-existent.

BREAKING_CHANGE_REASON=Removing legacy orphaned proto files that were left behind by OwlBot to ensure accurate 0-diff parity testing.

For googleapis/librarian#7366

@sofisl sofisl changed the title Chore/remove orphaned ads files chore(AdsAdManager): remove orphaned files from previous proto refactors Aug 20, 2026
@sofisl
sofisl marked this pull request as ready for review August 20, 2026 20:18
@sofisl
sofisl requested a review from a team as a code owner August 20, 2026 20:18
@sofisl

sofisl commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

For reviewers concerned about backwards compatibility:

Any customer code attempting to use these deleted classes is already broken and crashing in production.

Because the upstream .proto definitions were removed, the protobuf compiler previously removed these classes from the generated GPBMetadata descriptor pool. Because OwlBot left the PHP class files behind, they can still be referenced in code, but attempting to instantiate them immediately throws a Fatal PHP Exception before any API call is made.

Example instantiation of an orphaned class:

$request = new \Google\Ads\AdManager\V1\Report\DataTable();

Output:

PHP Fatal error:  Uncaught InvalidArgumentException: Google\Ads\AdManager\V1\Report\DataTable is not found in descriptor pool. Only generated classes may derive from Message.

Deleting these files simply cleans up the dead code and allows static analysis tools to correctly flag them as non-existent.

@Hectorhammett

Copy link
Copy Markdown
Collaborator

For future context and reference:
Sofia and I had an internal discussion regarding the problems of overriding a breaking change detector.

Removing obsolete classes it is considered a breaking change, 100%.

Currently we are holding on merging these changes as the PHP team is having a discussion on what approach to take, wether that being removing the outdated files or adding the outdated files.

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.

2 participants