Resolves a logging issue #3936
Resolves a logging issue #3936
Conversation
Closes dnnsoftware#3906 Apparently some files where deleted as part of dnnsoftware#3843 This PR brings those files back and also fixes stylecop warnings. Some class names had to be changed both for clarity and to respect stylecop rule that the file name should match the class name. Because of that change, the dnn manifest was also changed so it all works with the new names.
|
Looks good to me. Thanks for tracking this down @valadas! Was any functionality broken by these going missing? Do we even need these classes? I feel like it's a warning to look closer if we can lose a handful of classes and the only symptom is stuff filling the logs… |
|
@bdukes Well, each of these controllers control a menu item, from the looks of it it controls the visibility per role and on some it injects settings that I guess trickle down to the frontend initialization code. Those controllers are defined in the manifest and the logging issue was caused by them being defined but missing and reflection was choking trying to find them. It looks like in the case of not being defined in the manifest, it defaults to show to hosts only, so I guess for those who don't do anything else, it would be safe to remove, I decided to keep them because this is a bit cryptic to many people (including me) and I though it safer to leave it set as it was to prevent a possible change elsewhere that would change that default behaviour and expose menu items to users that should not see them... |
|
Yeah, in terms of this PR as a bug fix, just reverting this change is definitely the right approach. I'm just wondering out loud if 1️⃣ this is not cryptic for anyone, and 2️⃣ if it's worth a closer look to see if this makes sense or not. |
There was a problem hiding this comment.
Yes, thanks for tracking these down @valadas - are there any "lessons learned" from the mass stylecop implementations? Or are we now beyond that and looking at more small chunks of manual implementations?
Closes #3906
Apparently some files where deleted as part of #3843 which caused the extra logging.
This PR brings those files back and also fixes stylecop warnings. Some class names had to be changed both for clarity and to respect the stylecop rule that the file name should match the class name. Because of that change, the dnn manifest was also changed so it all works with the new names.