-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
Fix reflection based destructuring for redefined property #344
Conversation
@RehanSaeed if you think this is good, please release a new version, I lost track of how to do that :-). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made a small suggestion.
@RehanSaeed can we merge this? |
Sorry, will take a look on Monday. |
Thanks, in the meantime I will start working on next PR based on this one 👍 |
private readonly Exception exception; | ||
private readonly IExceptionPropertyFilter? filter; | ||
private readonly Dictionary<string, object?> properties = new(); | ||
private readonly Dictionary<string, object?> properties = new (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got some warings because:
The keyword 'new' should not be followed by a space
If you do not care I would rather merge this and then consider #347 separately. |
I may prepare another fix to tackle the problem of gathering properties from a type taking into account possible redefinitions.
However, I consider the change proposed here good practice for library robustness.
Fixes #343