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

InputCommon: Migrate logging over to fmt #9184

Merged
merged 1 commit into from Oct 23, 2020

Conversation

lioncash
Copy link
Member

Continues the migration of the logging calls over to the fmt capable ones.

@iwubcode
Copy link
Contributor

I haven't kept up with this. Just curious but why not just change the base log function and then search/replace the ones that broke or are non-conforming? Not super easy I guess?

Do you plan on removing the non FMT version after all the upgrades are finished?

@lioncash
Copy link
Member Author

lioncash commented Oct 22, 2020

Because that would be a huge PR that touches everything all at once, which sucks to review. Having both logging paths exist at once and migrating them over to the new one allows individual PRs to be kept compact and easily reviewed quickly and more thoroughly. I'd rather people not get mentally checked out halfway through a hypothetical 1000+ line PR that almost entirely consists of text formatting.

Yes the _FMT version will eventually have the suffix dropped once everything is moved over and the old one will be removed. There's no reason to keep two logging systems in place.

@lioncash lioncash force-pushed the inputlog branch 2 times, most recently from 2d8891f to 16fd117 Compare October 22, 2020 22:54
Copy link
Member

@BhaaLseN BhaaLseN left a comment

Choose a reason for hiding this comment

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

Just two tiny nits, up to you if you want to address them or not. LGTM.

Continues the migration of the logging calls over to the fmt capable
ones.
@lioncash
Copy link
Member Author

@BhaaLseN resolved them both

@leoetlino leoetlino merged commit ce6eda7 into dolphin-emu:master Oct 23, 2020
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants