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

revert using directive to control mock device and port #330

Merged

Conversation

patrickmcquay
Copy link
Contributor

per: 5f47e14#r124815628

I've just reverted this, there isnt really a better way to do this than was there if DEBUG is not usable.

additional #defines (MOCKPORT) would have to be removed before submission anyway, changing the project file to add different symbols or build configs is similar in that they would likely have to be removed before submission as well.

@patrickmcquay
Copy link
Contributor Author

also the mock code is of limited usability anyway. I'll probably grab a spare PCM to have a bench setup and a backup sooner or later.

@Gampyg28
Copy link
Collaborator

Why would #if MOCKPORT ... #endif need to be removed before submission ?

@patrickmcquay
Copy link
Contributor Author

It wouldnt, but the coresponding define would need to be, either that or the change to the project file to define it globally. Unless I'm missing something? Ive really not done a whole lot of work with preprocessor stuff, and couldnt think of anything else that would be as "automatic" as using the debug symbol.

Copy link
Collaborator

@antuspcm antuspcm left a comment

Choose a reason for hiding this comment

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

Yep, happy for this to be commented out. I dont think the mockport, or the tests, really got up to the desired standard. If anyone sees this and wants to implement tests and more tightly integrate mock port please do.

@antuspcm antuspcm merged commit 62928fc into LegacyNsfw:develop Aug 19, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants