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: Utility methods to support google-datetime format (and eventually others) #2432

Merged
merged 1 commit into from
Jun 16, 2023

Conversation

jskeet
Copy link
Collaborator

@jskeet jskeet commented Jun 16, 2023

Movement towards #2040, but doesn't finish fixing it yet.

@jskeet jskeet requested a review from a team as a code owner June 16, 2023 09:29
@jskeet
Copy link
Collaborator Author

jskeet commented Jun 16, 2023

Note: only the first commit will be merged. The second is for demonstration purposes.

Copy link
Contributor

@amanda-tarafa amanda-tarafa left a comment

Choose a reason for hiding this comment

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

LGTM. One question, I'm not even sure about that though.

private string _timestampRaw;

private object _timestamp;

/// <summary>The timestamp when this issue appeared.</summary>
[Newtonsoft.Json.JsonPropertyAttribute("timestamp")]
Copy link
Contributor

Choose a reason for hiding this comment

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

If we were to move this one to TimestampDateTimeOffset (by specifying to use GetDateTimeOffsetFromString to serialize it) would that give us the failure right before performing the request instead of having to wait for the server to do that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think so - but we'd have to also add a converter (either specified in the property or in general) to make sure that the DateTimeOffset is serialized correctly. It would work, but there'd be more risk of problems elsewhere IMO.

@jskeet jskeet force-pushed the google-format-utilities branch from 4057010 to 0025082 Compare June 16, 2023 11:14
@jskeet jskeet merged commit ec9c0ee into googleapis:main Jun 16, 2023
amanda-tarafa added a commit to amanda-tarafa/google-api-dotnet-client that referenced this pull request Jun 16, 2023
Features

- googleapis#2432 More utilities for date/time parsing.
amanda-tarafa added a commit that referenced this pull request Jun 16, 2023
Features

- #2432 More utilities for date/time parsing.
amanda-tarafa added a commit to amanda-tarafa/google-api-dotnet-client that referenced this pull request Jun 26, 2023
Updates support version: 1.61.0 - beta02 -> 1.61.0

Features

- Improvements for  date/time parsing.
  - googleapis#2441
  - googleapis#2432
  - googleapis#2429
- googleapis#2435 PKCE support.
- googleapis#2394 Which improves impersonation support.
- googleapis#2349 and googleapis#2379 Which improve error reported when ADC is not configured.
amanda-tarafa added a commit that referenced this pull request Jun 26, 2023
Updates support version: 1.61.0 - beta02 -> 1.61.0

Features

- Improvements for  date/time parsing.
  - #2441
  - #2432
  - #2429
- #2435 PKCE support.
- #2394 Which improves impersonation support.
- #2349 and #2379 Which improve error reported when ADC is not configured.
@jskeet jskeet deleted the google-format-utilities branch November 8, 2023 08:43
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