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

Remove mvvmlight dependency #504

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

laurentkempe
Copy link
Owner

No description provided.

Comment on lines 98 to 101
[SuppressMessage("Microsoft.Performance", "CA1822:MarkMembersAsStatic",
Justification = "The this keyword is used in the Silverlight version")]
[SuppressMessage("Microsoft.Design", "CA1030:UseEventsWhereAppropriate",
Justification = "This cannot be an event")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

❔ Are these still needed?

Copy link
Owner Author

Choose a reason for hiding this comment

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

WOW you are so fast to react 💨
Good point @sharwell I will have a second 👁👁 on it

Copy link
Owner Author

Choose a reason for hiding this comment

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

✅ Removed

Comment on lines +16 to +18
public virtual void Cleanup()
{
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

❔ Is this used?

Copy link
Owner Author

@laurentkempe laurentkempe Oct 7, 2021

Choose a reason for hiding this comment

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

Yeah this is used!


protected void RaisePropertyChanged([CallerMemberName] string propertyName = null)
{
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
Copy link
Collaborator

Choose a reason for hiding this comment

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

💡 If this is called a large number of times, consider caching the PropertyChangedEventArgs instances in a concurrent dictionary.

@laurentkempe laurentkempe force-pushed the remove-mvvmlight-dependency branch from 1f51c25 to 74d3e15 Compare October 7, 2021 18:28
@laurentkempe laurentkempe self-assigned this Oct 12, 2021
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