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

Internalize componentparameter #1111

Merged
merged 5 commits into from
Jun 16, 2023
Merged

Internalize componentparameter #1111

merged 5 commits into from
Jun 16, 2023

Conversation

egil
Copy link
Member

@egil egil commented Jun 8, 2023

Closes #1017

Merge after #1110, forgot to split the branches on the plane :)

@egil egil force-pushed the internalize-componentparameter branch from 161d9c5 to bed96a2 Compare June 8, 2023 15:11
@egil egil requested a review from linkdotnet June 8, 2023 15:16
MIGRATION.md Outdated Show resolved Hide resolved
@linkdotnet
Copy link
Collaborator

For sure the documentation and samples have to be updated.

@egil egil force-pushed the internalize-componentparameter branch 2 times, most recently from f91e789 to 145e708 Compare June 8, 2023 15:50
@egil
Copy link
Member Author

egil commented Jun 8, 2023

Thanks for the input. I'll be back with new changes.

@egil egil force-pushed the internalize-componentparameter branch from 62e1c64 to ff2f95d Compare June 10, 2023 09:08
@egil
Copy link
Member Author

egil commented Jun 14, 2023

Back on a plane, thus I finally have time to finish this. Docs should be up to date now.

@linkdotnet
Copy link
Collaborator

The sample solution is not compilable.
There are several instances of using static Bunit.ComponentParameterFactory;, also some ambiguous usings of Render and unresolvable types (AllKindsOfParamsTest for example).

@egil
Copy link
Member Author

egil commented Jun 15, 2023

The sample solution is not compilable. There are several instances of using static Bunit.ComponentParameterFactory;, also some ambiguous usings of Render and unresolvable types (AllKindsOfParamsTest for example).

That should be fixed now.

@linkdotnet
Copy link
Collaborator

Right now every test in AllKindsOfParamsTest is commented out. We could then just delete the file completely if those tests don't make sense anymore.

@egil
Copy link
Member Author

egil commented Jun 16, 2023

Right now every test in AllKindsOfParamsTest is commented out. We could then just delete the file completely if those tests don't make sense anymore.

Honestly was a bit lazy here. The problem is that the code snippets in the docs are referenced by line numbers meaning I would need to go and update all the line offsets, and that a fairly big task 😅

@linkdotnet
Copy link
Collaborator

linkdotnet commented Jun 16, 2023

Right now every test in AllKindsOfParamsTest is commented out. We could then just delete the file completely if those tests don't make sense anymore.

Honestly was a bit lazy here. The problem is that the code snippets in the docs are referenced by line numbers meaning I would need to go and update all the line offsets, and that a fairly big task 😅

I don't think that file is used. See here: https://grep.app/search?q=AllKindsOfParamsTest&case=true&filter[lang][0]=C%23

So maybe just delete the thing completely.

EDIT: The Passing parameters to components section is using different components for its content. So that file seems either for super advanced users that might check the repository and discover this - or it's obsolete. I am leaning towards the second opinion. So delete the file entirely

@egil egil force-pushed the internalize-componentparameter branch from 407006f to a9dc0d2 Compare June 16, 2023 17:42
@egil
Copy link
Member Author

egil commented Jun 16, 2023

Right now every test in AllKindsOfParamsTest is commented out. We could then just delete the file completely if those tests don't make sense anymore.

Honestly was a bit lazy here. The problem is that the code snippets in the docs are referenced by line numbers meaning I would need to go and update all the line offsets, and that a fairly big task 😅

I don't think that file is used. See here: https://grep.app/search?q=AllKindsOfParamsTest&case=true&filter[lang][0]=C%23

So maybe just delete the thing completely.

EDIT: The Passing parameters to components section is using different components for its content. So that file seems either for super advanced users that might check the repository and discover this - or it's obsolete. I am leaning towards the second opinion. So delete the file entirely

You are right, thanks for keeping me honest :)

Lets get this merged, unless you spot other things?

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.

Remove all RenderComponent methods and replace them with additional overloads of Render method
2 participants