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

HW: Convert FileMonitor::Log into a class. #11042

Merged

Conversation

AdmiralCurtiss
Copy link
Contributor

Removes global state in FileMonitor.cpp.

@JosJuice
Copy link
Member

JosJuice commented Sep 9, 2022

Having to write FileMonitor::FileLogger feels a bit odd. The two names are ostensibly referring to the same thing, and yet they're nested. But that's just a minor complaint. The rest LGTM.

@AdmiralCurtiss
Copy link
Contributor Author

AdmiralCurtiss commented Sep 9, 2022

I mean, not wrong, but we already had the FileMonitor namespace, and it feels a bit dirty to just have a class in the global namespace. Suggestions?

(why do we not have all of our code in a DolphinEmu namespace or something, anyway?)

@dolphin-emu-bot
Copy link
Contributor

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • line-width-test on uberogl-lin-radeon: diff

automated-fifoci-reporter

@JosJuice
Copy link
Member

JosJuice commented Sep 9, 2022

Hm... Yeah, I guess using the global namespace isn't great. If you want to keep it the way it is, I'm okay with that.

@lioncash lioncash merged commit 804af55 into dolphin-emu:master Sep 10, 2022
11 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the global-state-file-monitor branch September 10, 2022 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants