Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add a FieldFormats static class for auto-populated fields #758

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

jskeet
Copy link
Collaborator

@jskeet jskeet commented Feb 8, 2024

Fixes b/324254628

@jskeet jskeet added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Feb 8, 2024
@jskeet jskeet requested a review from a team as a code owner February 8, 2024 15:18
/// <summary>
/// Generator methods for auto-populated fields.
/// </summary>
public static class FieldGenerators
Copy link
Collaborator

@amanda-tarafa amanda-tarafa Feb 8, 2024

Choose a reason for hiding this comment

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

FieldValueGenerators or ValueGenerators ? I think just Field is confusing, specially when quickly seen in generator code.

Is something other than value generation that we expect to be doing here? Validations are unlikely I guess. But maybe canonicalization? Just thinking whether we want FieldGenerators and the hypotethetical FieldCanonicalizations or simply FieldHelpers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to FieldFormats.


using System;

namespace Google.Api.Gax;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, this is fine.

(I'd really like if we had a base package that both Apiaries and Gax/GAPICs could use, we'd then could put this kind of thing there (and WithCancellationToken :)). But that's something for another day.)

@jskeet jskeet changed the title feat: Add a FieldGenerators static class for auto-populated fields feat: Add a FieldFormats static class for auto-populated fields Feb 9, 2024
@jskeet jskeet removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Feb 9, 2024
@jskeet jskeet merged commit f1d44a4 into googleapis:main Feb 9, 2024
5 checks passed
@jskeet jskeet deleted the field-generation branch February 9, 2024 07:52
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