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

1190 modes in subfolders #1396

Merged
merged 3 commits into from Jul 26, 2019

Conversation

@pmansukhani
Copy link
Contributor

commented Jul 25, 2019

Do I need more tests for the modes loaded from subfolders?

pmansukhani added some commits Jul 24, 2019

@jabdoa2
Copy link
Collaborator

left a comment

Looks good to me. Thanks for taking the time!

@@ -0,0 +1,14 @@
#config_version=5
mode:
start_events: start_mode1

This comment has been minimized.

Copy link
@jabdoa2

jabdoa2 Jul 25, 2019

Collaborator

Small nitpick: This should be start_mode6. Otherwise other tests will accidentally start this mode. Same for the other new modes

@jabdoa2

This comment has been minimized.

Copy link
Collaborator

commented Jul 25, 2019

Do I need more tests for the modes loaded from subfolders?

The test is sufficient. You could add a negative test that we do not find subfolders/more modes within another mode.

@pmansukhani

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2019

I'll fix both and also squash the commits.

The documentation would probably need to change as well. Would there be a separate issue created for that? How is it usually handled?

@jabdoa2

This comment has been minimized.

Copy link
Collaborator

commented Jul 25, 2019

Feel free to just update the configuration in mpf-docs. We rarely use issues in the docs (mostly just as a reminder for stuff to do later).

@pmansukhani

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2019

These changes are ready for merge @jabdoa2 . I'll work on the documentation at some point next week.

@jabdoa2 jabdoa2 added this to the 0.53 milestone Jul 26, 2019

@jabdoa2

This comment has been minimized.

Copy link
Collaborator

commented Jul 26, 2019

Awesome. Thanks!

@jabdoa2 jabdoa2 merged commit 2037135 into missionpinball:dev Jul 26, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.