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.

Sign up for GitHub

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

Jump to bottom

Add Logging to the Dependency Injection SafeGetTypes function #4455

Merged
merged 10 commits into from
Feb 8, 2021
Merged

Add Logging to the Dependency Injection SafeGetTypes function #4455

merged 10 commits into from
Feb 8, 2021

Conversation

mtrutledge
Copy link
Contributor

Fixes #4454

Summary

Added logging to the SafeGetTypes function. The function would successfully load the types safely, without crashing, but would eat any exceptions that were thrown about missing references. I have added an ILog parameter to the function to print out the missing types.

mitchelsellers
mitchelsellers approved these changes Feb 5, 2021
View reviewed changes
Copy link
Contributor

@mitchelsellers mitchelsellers left a comment

Choose a reason for hiding this comment

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

This looks awesome!

@donker donker added this to the 9.9.0 milestone Feb 5, 2021
valadas
valadas requested changes Feb 5, 2021
View reviewed changes
Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

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

Left 2 comments just to get other eyes on these concerns...

DNN Platform/DotNetNuke.DependencyInjection/Extensions/TypeExtensions.cs Outdated Show resolved Hide resolved
DNN Platform/DotNetNuke.DependencyInjection/Extensions/TypeExtensions.cs Outdated Show resolved Hide resolved
valadas
valadas approved these changes Feb 8, 2021
View reviewed changes
Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

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

Ok, so I added the dependency in our nuspec file and I am ok with these changes until we have something different for logging.

@bdukes
Copy link
Contributor

bdukes commented Feb 8, 2021

👍 Ready to merge (let's do a Squash Merge on this one) once the build is complete.

@valadas valadas merged commit e3a65cd into dnnsoftware:develop Feb 8, 2021
@bdukes bdukes mentioned this pull request Jun 28, 2023
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SkyeHoefling SkyeHoefling SkyeHoefling left review comments

@bdukes bdukes bdukes approved these changes