Skip to content

Added async implementations to the HtmlFormatter. #376

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

clrudolphi
Copy link
Contributor

🤔 What's changed?

Added async method implementations.

⚡️ What's your motivation?

Reqnroll will want to consume the async implementations in its implementation of Messages.

🏷️ What kind of change is this?

  • 🏦 Refactoring/debt/DX (improvement to code design, tooling, etc. without changing behaviour)

♻️ Anything particular you want feedback on?

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • [X ] I've changed the behaviour of the code
    • [ X] I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • [ X] I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

@clrudolphi clrudolphi requested a review from gasparnagy June 2, 2025 15:45
Copy link
Member

@gasparnagy gasparnagy left a comment

Choose a reason for hiding this comment

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

Do we need to maintain both the async and the sync methods? Would it be just enough to keep the async one?

public override void Write(char[] value)
{
Write(value, 0, value.GetLength(0));
}

public async Task WriteAsync(char[] value)
Copy link
Member

Choose a reason for hiding this comment

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

Based on the warning, this method should have been done anyway.

'JsonInHtmlWriter.WriteAsync(char[])' hides inherited member 'TextWriter.WriteAsync(char[])'. Use the new keyword if hiding was intended.

@mpkorstanje
Copy link
Contributor

Do we need to maintain both the async and the sync methods?

Your call. The only difference is the resulting version number. If sync is removed then it will be a major release, otherwise a minor.

@clrudolphi
Copy link
Contributor Author

clrudolphi commented Jun 3, 2025

Do we need to maintain both the async and the sync methods? Would it be just enough to keep the async one?

I put the [Obsolete] marker on the sync constructors.
We can release this as a minor and come back later to remove them completely.

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.

3 participants