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

Added in CLI arg support #75

Merged
merged 2 commits into from
Jul 3, 2016
Merged

Added in CLI arg support #75

merged 2 commits into from
Jul 3, 2016

Conversation

geekbozu
Copy link
Contributor

@geekbozu geekbozu commented Jul 3, 2016

Added autotest From CLI
Added in a force load ROM from CLI option (this is broken)

Changed openJSONConfig to Int type. Use, Check if file was properly opened programmatically.

Added autotest From CLI
Added in a force load ROM from CLI option
@geekbozu geekbozu mentioned this pull request Jul 3, 2016
@adriweb
Copy link
Member

adriweb commented Jul 3, 2016

Cool ; here is some feedback before merging:

  • Delete the //#include <qdir.h> line
  • AFAIK, you can replace all instances of QCoreApplication::translate("main", foo); by a simpler tr(foo) ; edit: nope, nvm
  • Can't the sleep(1) rather be QThread::sleep(1)?
  • Technically the CEMUOpts type would be better as CEmuOpts :P
  • Rename TestFile to AutotesterFile?
  • The diff shows a few new empty lines (here, here, here, and here), you can delete those
  • Actually, I don't think there should be a suppressTestDialog thing ; indeed if we launch CEmu with a test config file, we probably want to know about the outcome. What I can do however, is simply not show the dialog in case of 0 tests to be done (but I'll take care of that later, don't worry about it in this PR).

@jacobly0
Copy link
Member

jacobly0 commented Jul 3, 2016

Also #include "QString" should be #include <QString>.

Added suppressTestDialog switch instead of relying on autotesterfile being present
@geekbozu
Copy link
Contributor Author

geekbozu commented Jul 3, 2016

Updated PR with suggestions.
Added in a seperate flag for suppressTestDialog

@adriweb adriweb merged commit 711b1fa into CE-Programming:master Jul 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants