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

Memory leaks fix and added hero image and circle cropped picture feature #77

Merged
merged 5 commits into from
Apr 8, 2023
Merged

Conversation

shaovoon
Copy link
Contributor

@shaovoon shaovoon commented Feb 15, 2022

Dear Mohabouje,

I have merged my memory leaks fix and added hero image and circle cropped picture in this PR. Read about the memory leak fix in my article: https://www.codeproject.com/Articles/5286393/Cplusplus-Windows-Toast-Notification

The main memory leaks are in activatedToken, dismissedToken and failedToken which have to be removed after each callback.

There is a breaking change in showToast()'s handler parameter type which is changed from raw pointer to shared_ptr.

I want to thank you and your contributors on the behalf of all Windows programmers for writing and giving us WinToast. Thanks for this excellent library!

@mohabouje
Copy link
Owner

Amazing contribution! I'm a little bit busy in the coming days but I will have a look later this week and prepare a new release with the changes.

@shaovoon
Copy link
Contributor Author

shaovoon commented Feb 15, 2022

If you have any questions about my code and its rationale for doing so, please feel free to ask me here. It is faster to ask me than to figure it out by yourself. I am in Singapore timezone, so I may not respond ASAP.

If there is any code that you do not agree with my implementation, I'll suggest that you merge my PR first and do the refactoring yourself rather than you and me going back and forth on how to edit my PR.

EDIT: Please kindly remember to document the changed showToast() in your README when you're making the new release.

EDIT: To understand my memory leak fix: read it here.

Thanks for reading.

@shaovoon
Copy link
Contributor Author

I have added SAL annotations and followed WinToast coding conventions for my code in the latest commit.

And showToast() is reverted to use IWinToastHandler raw pointer.

@shaovoon
Copy link
Contributor Author

Dear Mohabouje,

It has been more than 2 months and this PR is still not approved. Have you reviewed the code yet?

@nickdademo
Copy link

@mohabouje Any plans to review and merge this soon?

@Paulchen-Panther
Copy link

@mohabouje It would be nice if this PR would be merged.

@mohabouje mohabouje merged commit 1135eef into mohabouje:master Apr 8, 2023
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

4 participants