Skip to content

chore: Map time.Duration to TimeSpan everywhere#98

Merged
HofmeisterAn merged 8 commits intotestcontainers:mainfrom
campersau:timespanbytes
Apr 20, 2026
Merged

chore: Map time.Duration to TimeSpan everywhere#98
HofmeisterAn merged 8 commits intotestcontainers:mainfrom
campersau:timespanbytes

Conversation

@campersau
Copy link
Copy Markdown

@campersau campersau commented Apr 17, 2026 •
edited
Loading

Uses TimeSpanNanosecondsConverter for all time.Duration fields.
Use byte[] instead of List<byte> which JsonSerializer supports natively so Base64Converter can be removed.

campersau
campersau commented Apr 17, 2026
View reviewed changes
Comment thread src/Docker.DotNet/JsonTimeSpanSecondsConverter.cs
HofmeisterAn
HofmeisterAn reviewed Apr 18, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@HofmeisterAn HofmeisterAn left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, thanks! My only suggestion is to align the converter names, right now they're not consistent. WDYT?

  • ConsoleSizeConverter
  • JsonDateTimeConverter
  • JsonEnumMemberConverter
  • JsonTimeSpanNanoSecondsConverter (Nanoseconds)
  • TimeSpanSecondsConverter

HofmeisterAn
HofmeisterAn reviewed Apr 18, 2026
View reviewed changes
Comment thread src/Docker.DotNet/JsonTimeSpanNanoSecondsConverter.cs Outdated
@campersau
Copy link
Copy Markdown
Author

campersau commented Apr 18, 2026

LGTM, thanks! My only suggestion is to align the converter names, right now they're not consistent. WDYT?

  • ConsoleSizeConverter
  • JsonDateTimeConverter
  • JsonEnumMemberConverter
  • JsonTimeSpanNanoSecondsConverter (Nanoseconds)
  • TimeSpanSecondsConverter

I did rename them. All have now a Json prefix.

Should I rename these?

  • BoolQueryStringConverter -> QueryStringBoolConverter
  • EnumerableQueryStringConverter -> QueryStringEnumerableConverter

Could also be in a follow up PR.

@HofmeisterAn
Copy link
Copy Markdown
Collaborator

HofmeisterAn commented Apr 19, 2026

Should I rename these?

  • BoolQueryStringConverter -> QueryStringBoolConverter
  • EnumerableQueryStringConverter -> QueryStringEnumerableConverter

Sounds good 👍

Could also be in a follow up PR.

Whatever you prefer.

@HofmeisterAn HofmeisterAn added the enhancement New feature or request label Apr 19, 2026
@HofmeisterAn HofmeisterAn changed the title Map time.Duration to TimeSpan everywhere chore: Map time.Duration to TimeSpan everywhere Apr 19, 2026
HofmeisterAn
HofmeisterAn previously approved these changes Apr 19, 2026
View reviewed changes
@campersau
Copy link
Copy Markdown
Author

campersau commented Apr 19, 2026

I renamed the QueryStringConverters as well.

@HofmeisterAn HofmeisterAn merged commit 22b4f19 into testcontainers:main Apr 20, 2026
15 checks passed
@campersau campersau deleted the timespanbytes branch April 20, 2026 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@HofmeisterAn HofmeisterAn HofmeisterAn approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@campersau @HofmeisterAn