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

Update 18_shots.rst #253

Merged
merged 1 commit into from Aug 11, 2019

Conversation

@ironspider
Copy link
Contributor

commented Aug 11, 2019

A couple grammar fixes but mostly suggesting changes based on the confusion created by this page originally referencing the machine-wide config and many of those things still existing (instead of referencing base.yaml which is the new way of doing things). I found myself confused several times going through this page of the tutorial because it would keep saying "machine-wide config" when it meant "base.yaml". So I've made suggestions to update it all and get it in line with the base.yaml approach. Here are some examples:

Around line 55 it says to "open the base.yaml" file but you are already in that file since the tutorial no longer has you working in the machine-wide config.

Around line 85 it references two config files but in the current way of doing things you would only have one file (base.yaml) needing to be saved.

Around line 587 it references machine-wide config file but you're actually working in base.yaml here.

Around line 620 it makes reference to the priority level of the machine-wide config file and this is a tad confusing as you're not working in that file anymore (you're working in base.yaml). So that statement kind of conflicts with the previous sentence in the tutorial (which is about base.yaml). And, while it's good to know that the machine-wide config runs at a priority of 0, that is covered in the Modes section previously so I think it's better to just remove it?

I know that some of these writings are to be able to support people working on multiple versions but I think if we no longer support shot definition in the machine-wide config we should make this section as modern and connected as possible. For me, this was the first time in the tutorial I felt a bit confused and/or frustrated and since it's such an important topic I think this tutorial page has to be really dialed in.

Update 18_shots.rst
A couple grammar fixes but mostly suggesting changes based on the confusion created by this page originally referencing the machine-wide config and many of those things still existing (instead of referencing base.yaml which is the new way of doing things). I found myself confused several times going through this page of the tutorial because it would keep saying "machine-wide config" when it meant "base.yaml". So I've made suggestions to update it all and get it in line with the base.yaml approach. Here are some examples:

Around line 55 it says to "open the base.yaml" file but you are already in that file since the tutorial no longer has you working in the machine-wide config.

Around line 85 it references two config files but in the current way of doing things you would only have one file (base.yaml) needing to be saved.

Around line 587 it references machine-wide config file but you're actually working in base.yaml here.

Around line 620 it makes reference to the priority level of the machine-wide config file and this is a tad confusing as you're not working in that file anymore (you're working in base.yaml). So that statement kind of conflicts with the previous sentence in the tutorial (which is about base.yaml). And, while it's good to know that the machine-wide config runs at a priority of 0, that is covered in the Modes section previously so I think it's better to just remove it?

I know that some of these writings are to be able to support people working on multiple versions but I think if we no longer support shot definition in the machine-wide config we should make this section as modern and connected as possible. For me, this was the first time in the tutorial I felt a bit confused and/or frustrated and since it's such an important topic I think this tutorial page has to be really dialed in.
@jabdoa2

This comment has been minimized.

Copy link
Collaborator

commented Aug 11, 2019

Thanks for taking the time to improve the tutorial. Those are the things which make a documentation really great. Appreciate it!

@jabdoa2 jabdoa2 merged commit 69b141f into missionpinball:dev Aug 11, 2019

1 check passed

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.