Skip to content

fix: suppress warnings on enumeration of deprecated fields - #990

Draft
robertvoinescu-work wants to merge 1 commit into
googleapis:mainfrom
robertvoinescu-work:fix/obsoletePagination
Draft

fix: suppress warnings on enumeration of deprecated fields#990
robertvoinescu-work wants to merge 1 commit into
googleapis:mainfrom
robertvoinescu-work:fix/obsoletePagination

Conversation

@robertvoinescu-work

Copy link
Copy Markdown
Contributor

b/543020713

@robertvoinescu-work
robertvoinescu-work requested a review from a team as a code owner August 5, 2026 21:22

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

Copy link
Copy Markdown

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 introduces support for disabling obsolete warnings on properties generated for deprecated response resource fields in paginated methods. It adds extension methods to PropertyDeclarationSyntax for handling pragma warnings, tracks whether a response resource field is deprecated in MethodDetails, and applies the pragma warning disable in ServiceCodeGenerator. The feedback suggests breaking down a long statement in ServiceCodeGenerator.cs into a local variable to improve readability.

Comment on lines 72 to +74
var propertyName = method.ResourcesFieldName;
var genericGetEnumerator = Method(Public, ctx.Type(Typ.Generic(typeof(IEnumerator<>), method.ResourceTyp)), "GetEnumerator")()
.WithBody(Property(Public, ctx.TypeDontCare, propertyName).Call(nameof(IEnumerable<int>.GetEnumerator))())
.WithBody(Property(Public, ctx.TypeDontCare, propertyName).MaybeWithPragmaDisableObsoleteWarning(method.ResponseResourceFieldIsDeprecated).Call(nameof(IEnumerable<int>.GetEnumerator))())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The statement for genericGetEnumerator is extremely long (exceeding 180 characters) and hard to read. Breaking it down by introducing a local variable for the property expression significantly improves readability and maintainability.

                        var propertyName = method.ResourcesFieldName;
                        var property = Property(Public, ctx.TypeDontCare, propertyName)
                            .MaybeWithPragmaDisableObsoleteWarning(method.ResponseResourceFieldIsDeprecated);
                        var genericGetEnumerator = Method(Public, ctx.Type(Typ.Generic(typeof(IEnumerator<>), method.ResourceTyp)), "GetEnumerator")()
                            .WithBody(property.Call(nameof(IEnumerable<int>.GetEnumerator))())

@amanda-tarafa amanda-tarafa 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.

Please add tests for this.

@amanda-tarafa
amanda-tarafa marked this pull request as draft August 5, 2026 21:58
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