Skip to content

Conversation

@mikeebowen
Copy link
Collaborator

@github-actions
Copy link

github-actions bot commented Mar 19, 2025

Test Results

    57 files  ±  0      57 suites  ±0   54m 36s ⏱️ + 1m 4s
 2 050 tests +  8   2 047 ✅ +  8   3 💤 ±0  0 ❌ ±0 
32 177 runs   - 121  32 141 ✅  - 121  36 💤 ±0  0 ❌ ±0 

Results for commit 0ec2368. ± Comparison against base commit f990ae8.

♻️ This comment has been updated with latest results.

@mikeebowen mikeebowen enabled auto-merge (squash) March 20, 2025 21:01
Copy link
Member

@twsouthwick twsouthwick left a comment

Choose a reason for hiding this comment

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

Looking good - with this new settings object, it can be made available everywhere, except that the Async property would be around the if/def

twsouthwick
twsouthwick previously approved these changes Mar 26, 2025
Copy link
Member

@twsouthwick twsouthwick left a comment

Choose a reason for hiding this comment

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

Just a nit, but overall good!

@mikeebowen mikeebowen requested a review from twsouthwick March 26, 2025 21:52
Copy link
Member

@twsouthwick twsouthwick left a comment

Choose a reason for hiding this comment

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

Take a look at the docs, then it's good to merge

{
#if FEATURE_ASYNC_SAX_XML
/// <summary>
/// Gets or sets a value indicating whether asynchronous OpenXmlPartWriter methods can be used.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Gets or sets a value indicating whether asynchronous OpenXmlPartWriter methods can be used.
/// Gets or sets a value indicating whether asynchronous <see cref="OpenXmlPartWriter" /> methods can be used.

#endif

/// <summary>
/// Gets or sets a value indicating whether the OpenXmlPartWriter should check to ensure that all characters in the document conform to the "2.2 Characters" section of the W3C XML 1.0 Recommendation.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Gets or sets a value indicating whether the OpenXmlPartWriter should check to ensure that all characters in the document conform to the "2.2 Characters" section of the W3C XML 1.0 Recommendation.
/// Gets or sets a value indicating whether the <see cref="OpenXmlPartWriter" /> should check to ensure that all characters in the document conform to the "2.2 Characters" section of the W3C XML 1.0 Recommendation.

Do you have a link to that part of the docs?

Copy link
Member

Choose a reason for hiding this comment

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

Is that really what the "CloseOutput" property does?

public bool CloseOutput { get; set; }

/// <summary>
/// Gets or sets a value indicating whether the OpenXmlPartWriter should also close the underlying stream or TextWriter when the Close() method is called.
Copy link
Member

Choose a reason for hiding this comment

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

I think these summaries are off...

@mikeebowen mikeebowen merged commit 6847874 into dotnet:main Mar 27, 2025
22 checks passed
@mikeebowen mikeebowen deleted the add-async-xmlwriter-methods-913 branch March 27, 2025 20:36
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.

Writing asynchronously with OpenXmlWriter

3 participants