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

Add ComponentParameterCollectionBuilder overloaded methods #1467

Merged
merged 2 commits into from
May 28, 2024

Conversation

Qwertyluk
Copy link

@Qwertyluk Qwertyluk commented May 10, 2024

Closes #751

note: I haven't updated CHANGELOG.md because I couldn't find an appropriate section for the changes for v2.

PR meta checklist

  • Pull request is targeted at main branch for code
    or targeted at stable branch for documentation that is live on bunit.dev.
  • Pull request is linked to all related issues, if any.
  • I have read the CONTRIBUTING.md document.

Code PR specific checklist

  • My code follows the code style of this project and AspNetCore coding guidelines.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • I have updated the appropriate sub section in the CHANGELOG.md.
  • I have added, updated or removed tests to according to my changes.
    • All tests passed.

@Qwertyluk Qwertyluk changed the title Add ComponentParameterCollectionBuilder overload methods Add ComponentParameterCollectionBuilder overloaded methods May 10, 2024
@linkdotnet
Copy link
Sponsor Collaborator

linkdotnet commented May 13, 2024

Hey @Qwertyluk - thank you for the PR.
The change itself looks good. I have two small amendments:

  1. Could you add a very small example with an explanation in the docs (https://bunit.dev/docs/providing-input/passing-parameters-to-components.html?tabs=csharp)? This will help also users why one would need such a function.
  2. Can you add your contribution in the CHANGELOG.md? Under the Unreleased section add another Added with a small text crediting you (I guess you will get the gist if you read some others).

@egil
Copy link
Member

egil commented May 14, 2024

If you just update the changelog like Steven says, then our deployment will add the correct version tag next to it.

@Qwertyluk
Copy link
Author

I've updated the changelog.

Though I haven't touched the documentation as I'm not sure if these changes are significant enough to require documentation updates.
There was already an overloaded method ComponentParameterCollectionBuilder.Add that supports asynchronous callbacks and accepts Func<Task>. In this PR, I only added the ability to pass arguments for async callbacks so I have a feeling that mentioning these overloads might unnecessarily complicate the documentation.
However, if you believe the documentation should be updated to include the support for asynchronous callback parameters, let me know and I'll make the necessary updates.

@linkdotnet
Copy link
Sponsor Collaborator

Sorry for the delay. LGTM. Will run CI

@egil egil merged commit 4ed4747 into bUnit-dev:v2 May 28, 2024
10 checks passed
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.

None yet

3 participants