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

Flow chart is not entirely representing for loop #408

Open
wants to merge 1 commit into
base: gh-pages
from

Conversation

Projects
None yet
4 participants
@lexnederbragt
Member

lexnederbragt commented Jun 20, 2016

I find the flow chart part of the shell_script_for_loop_flow_chart in 04-loop not really representing a for-loop in that it says $filename found --> No --> End? If $filename is not found, the shell will throw an error.

This PR changes the text to Next $filename? in the hope that would work better.

(I don't know how to recreate the vsdfile)

Flow chart is not entirely representing for loop
I find the flow chart part of the [shell_script_for_loop_flow_chart](https://github.com/swcarpentry/shell-novice/blob/gh-pages/fig/shell_script_for_loop_flow_chart.svg) in 04-loop not really representing a for-loop in that it says `$filename found --> No` --> `End`? If `$filename` is not found, the shell will throw an error.

This PR changes the text to `Next $filename?` in the hope that would work better.
@shwina

This comment has been minimized.

Member

shwina commented Jun 21, 2016

  1. As @lexnederbragt mentions, $filename found? isn't the right question to ask. Maybe the better question is "Are we done with all files?". What say?
  2. I'm not entirely convinced about the stdin. Shouldn't stdin be input-1.dat original-input-1.dat, etc.?
  3. Finally, the chart is created using Microsoft Visio. At the moment, we've got a copy of both the .vsd, and the corresponding .svg. This is a PR that updates only the .svg, which will make the .svg fall out of sync.

@fgacenga - copying you as the original contributor. Any suggestions?

@gvwilson gvwilson added bug labels Aug 2, 2016

rgaiacs pushed a commit to rgaiacs/swc-shell-novice that referenced this pull request May 6, 2017

Merge pull request swcarpentry#408 from mkuzak/mkuzak-patch1
Correct link to testimonials from instructors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment