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

Hotfix/5.12.18 #1841

Merged
merged 10 commits into from
Jul 29, 2021
Merged

Hotfix/5.12.18 #1841

merged 10 commits into from
Jul 29, 2021

Conversation

GeertvanHorrik
Copy link
Member

No description provided.

Copy link

@Averus-a Averus-a left a comment

Choose a reason for hiding this comment

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

Left some feedback, may be it will be helpful

src/Catel.Core/Configuration/ConfigurationService.cs Outdated Show resolved Hide resolved
src/Catel.Core/Configuration/ConfigurationService.cs Outdated Show resolved Hide resolved
/// </summary>
/// <param name="data">The payload data.</param>
/// <param name="tag">The optional Catel mediator tag to be used.</param>
public static void SendWith(TData data, object tag = null)

Choose a reason for hiding this comment

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

It's good to have in the future SendWith(TData data, Action<TMessage>)
Sometimes it's need to initialize some properties on the Message but it's required to introduce aggreagte class to pass it on TData place.
I'm usually using the non-static way to send message and adding depedency on mediator instead of using such helpers because of this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have introduced:

public static void SendWith(TData data, Action<TMessage> initializer, object tag = null)
{
    var message = With(data);

    if (initializer is not null)
    {
        initializer(message);
    }

    Send(message, tag);
}

Hopefully that is what you meant. If not, please let me know.

Copy link

@Averus-a Averus-a left a comment

Choose a reason for hiding this comment

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

Thank you very much!

@GeertvanHorrik GeertvanHorrik merged commit 2ca56db into develop Jul 29, 2021
@delete-merged-branch delete-merged-branch bot deleted the hotfix/5.12.18 branch July 29, 2021 07:57
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.

2 participants