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

Episode13 corrections #414

Merged

Conversation

juleskerley
Copy link
Contributor

The first commit is adding some clarification to make sure a reader doesn't believe ls is meant to read the file contents.

The second fixes a naming error with the job script. In all the snippets, the job-name given was "new_name". This fixes the issue of the job-name appearing as "hello-world" in the example before that.

There is an implication that `ls` can be used to read the file.
This disambiguates that implication.
Upon reviewing everyone's current job-with-name-status.snip, I realized that they all had "new_name" instead of "hello-world".
If "hello-world" is preferred, that would mean updating everyone's snip instead of the one line here.
@tkphd
Copy link
Contributor

tkphd commented Jul 19, 2022

Thanks for another contribution, @Kerleyark! The clarifications are appreciated.

I made the change to the script filename (new_name to hello-world) because learners and inexpert instructors are more likely to just type what's there than to come up with something on the fly, and "new name" is a terrible title. The fix for the discrepancy is to propagate this change through the snippet library (opened as issue #415), not to revert the job name.

If you feel comfortable making that change yourself, please feel free to do so as part of this PR. If not, please revert a38da5b and I'll open a new PR to resolve the new issue.

@juleskerley
Copy link
Contributor Author

Thank you, @tkphd. I'm glad to help.
I believe I can go through the snippets files and change the variable name from new_name to hello-world.
I will revert a38da5b shortly and have the snippets fixed.

Copy link
Contributor

@tkphd tkphd left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@tkphd tkphd merged commit 23df2f8 into carpentries-incubator:gh-pages Jul 21, 2022
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.

2 participants