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

chore(csharp): Misc improvements #5838

Merged
merged 7 commits into from
Feb 3, 2025
Merged

Conversation

Swimburger
Copy link
Member

@Swimburger Swimburger commented Feb 3, 2025

Description

Miscellaneous improvement for the C# generator

  • Call .ConfigureAwait(false) on Tasks
  • Use Enumerable<T>.Empty instead of creating a new empty list
  • Add PolySharp to test project and use C# 12
  • Remove redundant #nullable enable directives
  • Use SystemTask alias for System.Threading.Tasks.Task to avoid conflict with potentially named Task resources
  • Improve C# syntax

Testing

  • Seed

- Call `.ConfigureAwait(false)` on Tasks
- Use Enumerable<T>.Empty instead of creating a new empty list
- Add PolySharp to test project and use C# 12
- Improve C# syntax
- Remove redundant `#nullable enable` directives
@Swimburger Swimburger requested a review from dsinghvi as a code owner February 3, 2025 02:25
@Swimburger Swimburger changed the title internal(cshvarp): Misc improvements internal(csharp): Misc improvements Feb 3, 2025
Copy link

github-actions bot commented Feb 3, 2025

@Swimburger Swimburger changed the title internal(csharp): Misc improvements chore(csharp): Misc improvements Feb 3, 2025
Copy link
Member

@amckinney amckinney left a comment

Choose a reason for hiding this comment

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

Overall looks great, but a couple test definitions might need some attention (e.g. pagination).

"enum_property": "{{KnownEnumValue2String}}"
}
""";
private const string JsonWithKnownEnum2 = $$"""
Copy link
Member

Choose a reason for hiding this comment

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

Is this string format preferred? I would have thought the previous nesting was better.

Copy link
Member Author

Choose a reason for hiding this comment

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

This must be csharpier, the source template doesn't format it like this

@fern-api fern-api deleted a comment from fern-support Feb 3, 2025
@Swimburger
Copy link
Member Author

Overall looks great, but a couple test definitions might need some attention (e.g. pagination).

I think test definitions changed which is uncovering some bugs in the C# generator.

@Swimburger Swimburger merged commit d2b450f into main Feb 3, 2025
51 of 52 checks passed
@Swimburger Swimburger deleted the niels/csharp/misc-improvements branch February 3, 2025 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants