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

[net6.0][Core]Fixes wrong declared CommandMapper types #9930

Merged

Conversation

myroot
Copy link
Contributor

@myroot myroot commented Sep 6, 2022

Description of Change

Some Handlers are using missmatched types on CommandMapper. It makes me hard to add command mapper so I fix it

for example

public static CommandMapper<IActivityIndicator, IIndicatorViewHandler> CommandMapper = new(ViewCommandMapper)

IndicatorView must use IIndicatorView type as VirtualView type but it use IActivityIndicator,
I think, It seems mistake. if it is not mistake, let me know

@ghost ghost added the community ✨ Community Contribution label Sep 6, 2022
@ghost
Copy link

ghost commented Sep 6, 2022

Hey there @myroot! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@myroot myroot changed the title Fixes wrong declared CommandMapper types [net6.0][Core]Fixes wrong declared CommandMapper types Sep 6, 2022
@mattleibow
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@mattleibow
Copy link
Member

Can you retarget this pr to release/6.0.5xx-sr5? Then we can get into the upcoming release.

@mattleibow mattleibow added this to the .NET 6 + Servicing milestone Sep 6, 2022
@myroot myroot changed the base branch from net6.0 to release/6.0.5xx-sr5 September 7, 2022 01:17
@myroot
Copy link
Contributor Author

myroot commented Sep 7, 2022

Can you retarget this pr to release/6.0.5xx-sr5? Then we can get into the upcoming release.

Sure

@myroot myroot force-pushed the fix-wrong-command-mapper-type branch from 563663b to f0c85d8 Compare September 7, 2022 01:22
@mattleibow
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

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

@mattleibow i think the API changes are wrong no? we should add on unshipped ? the ***REMOVE and add the new API

@mattleibow
Copy link
Member

Ah yeah

@mattleibow
Copy link
Member

/azp run

@mattleibow mattleibow modified the milestones: .NET 6 + Servicing, 6.0-sr5 Sep 8, 2022
@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@mattleibow mattleibow merged commit c98a0a8 into dotnet:release/6.0.5xx-sr5 Sep 8, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants