first attempt at a new student lesson in swc-shell

[ACharbonneau]

I thought that the current student lesson for the shell started with a lot of definitions without giving much context for them, or obvious reasons for learning them. I've tried to fix that.

[ahmadia]

Thanks @ACharbonneau and welcome to the bc repository, where all the magic gets made :)

Just a quick sanity check before we review this. It looks like you're checking in an IPython Notebook named student_edition.ipynb, which seems like it might be a modified copy of shell-overview.ipynb in the same directory. Are you suggesting a fork of the shell-overview notebook, a replacement of it, something else?

[ACharbonneau]

Hi,

It is a based off of both the shell-overview and the shell-filedir-instructor.

I've seen on the mailing list a lot of discussion about whether there should be separate lesson plans for the students and teachers, or just one over-arching one, but I wasn't sure what had been decided as the desired format for lessons. In my imagination, this could be used by both the instructor and student, but I didn't fork one of the existing ones, because I wasn't sure if the 'single lesson' was the direction people had decided to go.

If it's becoming a fork or replacement, it should probably be of the shell-overview rather than the instructor, as it has a lot of explanation that the instructor presumably wouldn't require.

Amanda

On Oct 22, 2013, at 2:35 PM, Aron Ahmadia [email protected] wrote:

Thanks @ACharbonneau and welcome to the bc repository, where all the magic gets made :)

Just a quick sanity check before we review this. It looks like you're checking in an IPython Notebook named student_edition.ipynb, which seems like it might be a modified copy of shell-overview.ipynb in the same directory. Are you suggesting a fork of the shell-overview notebook, a replacement of it, something else?

—
Reply to this email directly or view it on GitHub.

[ahmadia]

Okay, before we need to merge this in, we'll need to think carefully about what material it's replacing, and how we can avoid any duplication of materials in the repository. If you are feeling confident, you might try using something like:

git rm shell-overview.ipynb
git mv student_edition.ipynb shell-overview.ipynb
git commit -m "proposed replacement for shell-overview"
git push # whatever command you're using here to push to your fork on GitHub

That will update this pull request with an amended version, where you are proposing that we replace shell-overview with your current content (it makes it much easier to see what you're removing and adding).

[ACharbonneau]

That seems reasonable. I've updated the pull request to replacing shell-overview.ipynb.

On Oct 22, 2013, at 2:49 PM, Aron Ahmadia [email protected] wrote:

Okay, before we need to merge this in, we'll need to think carefully about what material it's replacing, and how we can avoid any duplication of materials in the repository. If you are feeling confident, you might try using something like:

git rm shell-overview.ipynb
git mv student_edition.ipynb shell-overview.ipynb
git commit -m "proposed replacement for shell-overview"
git push # whatever command you're using here to push to your fork on GitHub
That will update this pull request with an amended version, where you are proposing that we replace shell-overview with your current content (it makes it much easier to see what you're removing and adding).

—
Reply to this email directly or view it on GitHub.

[ahmadia]

Perfect! I took a quick peek through and this looks great. This is a bigger PR, so please be patient as it's going to take some time to review.

[rgaiacs]

Since this is a shell tutorial what do you think of using /home/amanda/Desktop/Work_Stuff/Project_1/datasheets/super_important.xlsx that look like with Unix standards?

[ahmadia]

@r-gaia-cs - are you implying that OS X is not UNIXy enough for you :)

This one's not worth worrying about.

[rgaiacs]

@ahmadia I'm not a OS X user and still learning about it. =)

[rgaiacs]

@ACharbonneau You do a nice job in this PR.

You must take a look at this comment. The other are just "typos".

@ahmadia What do you think in merge all the IPython Notebook cells into one?

[ACharbonneau]

Thanks.

This is my first time using the comment system in git, and I want to make sure I'm following…the comment you pointed out is about sh vs bash? Or about the length of the lesson?

On Oct 22, 2013, at 6:04 PM, r-gaia-cs [email protected] wrote:

@ACharbonneau You do a nice job in this PR.

You must take a look at this comment. The other are just "typos".

@ahmadia What do you think in merge all the IPython Notebook cells into one?

—
Reply to this email directly or view it on GitHub.

[rgaiacs]

@ACharbonneau Is the one about grant_Proposal.doc directory.

[ACharbonneau]

Oh! I thought of that as a typo. I edited those as I saw the messages, so that is fixed in the "Fixed the copious spelling errors" push I just did.

On Oct 22, 2013, at 6:16 PM, r-gaia-cs [email protected] wrote:

@ACharbonneau Is the one about grant_Proposal.doc directory.

—
Reply to this email directly or view it on GitHub.

[rgaiacs]

I notice that.

Can I ask you to change back "once" with "twice" in line 351? I think that you didn't see this comment. Sorry about that.

[ahmadia]

this should probably be restored.

[ahmadia]

@ACharbonneau - You've clearly done this sort of thing before :) I'd still like to do a more detailed check of how this material fits in to the shell lessons that it's replacing, and this may end up being more work for you as we figure out how it's affecting the flow of the content around it.

Thanks again for your quick responses on the comments and great job so far.

[ahmadia]

@r-gaia-cs - Thanks again for your in-depth review. It's relatively quick to spot syntax/grammar errors but I really appreciate you taking the time to look in to some of the tougher details of consistency, flow, and summarization.

[ACharbonneau]

That's fine. I wasn't expecting such a quick review. I'd be happy to edit this or other sections as necessary to make them more 'user friendly' and/or improve fit. I've tutored for/helped teach several intro to computing classes, and I've always found that the higher level people tend to figure out what's going on in spite of the teaching, and even if it's not put in context for them. But the lower level people get frustrated and lost really quickly. I have a few other re-writes in mind for other lessons that I've successfully used in person to help people understand what was going on.

On Oct 22, 2013, at 6:57 PM, Aron Ahmadia [email protected] wrote:

@ACharbonneau - You've clearly done this sort of thing before :) I'd still like to do a more detailed check of how this material fits in to the shell lessons that it's replacing, and this may end up being more work for you as we figure out how it's affecting the flow of the content around it.

Thanks again for your quick responses on the comments and great job so far.

—
Reply to this email directly or view it on GitHub.

[ahmadia]

I'm flagging this as needs-review, thanks for your patience on this @ACharbonneau.

[gvwilson]

I think we should merge this, then @gvwilson and @ethanwhite will decide whether this material goes into the shell/novice and/or shell/intermediate lessons. Any votes again?

[ACharbonneau]

Without really knowing how you guys are planning to run the division, I would say it belongs in shell/novice

On Nov 8, 2013, at 3:57 PM, Greg Wilson [email protected] wrote:

I think we should merge this, then @gvwilson and @ethanwhite will decide whether this material goes into the shell/novice and/or shell/intermediate lessons. Any votes again?

—
Reply to this email directly or view it on GitHub.

[ahmadia]

I don't think this should go in without a review of how it impacts lessons. @gvwilson - I understand you're eager to start getting material into novice and intermediate, but we can't do that at the expense of breaking existing material.

[gvwilson]

On 2013-11-08 4:09 PM, Aron Ahmadia wrote:

I don't think this should go in without a review of how it impacts
|lessons|. @gvwilson https://github.com/gvwilson - I understand
you're eager to start getting material into |novice| and
|intermediate|, but we can't do that at the expense of breaking
existing material.

I don't see the breakage?

[ahmadia]

@gvwilson - This PR substantially overhauls shell-overview.ipynb. To my knowledge, these changes haven't been reviewed yet.

[ethanwhite]

On Nov 8, 2013 3:59 PM, "ACharbonneau" [email protected] wrote:

Without really knowing how you guys are planning to run the division, I
would say it belongs in shell/novice

I'm actually not clear myself on what shell/intermediate is going to
include. I think this will benefit from more conversation in a separate
issue.

[ahmadia]

This PR has been on my guilt list for the last two weeks. Does anybody have some time to review this?

[ahmadia]

(Okay, I'm reviewing this one today)

[ahmadia]

This PR provides an example IPython Notebook for teaching the shell lesson on file directories.

The notebooks originally present in this directory were an (almost-empty) overview notebook and a instructor notebook for teaching shell, files, and directories.

This PR removes the two earlier notebooks and provides a single notebook for presenting the lesson. The current file name is shell-overview.ipynb, but it makes sense to rename this notebook to shell-filedir.ipynb. I apologize for my earlier confusion in the PR about file contents and naming. I'll refer to the material by its current name for the remainder of the review.

Duplicated Material

shell-overview.ipynb is mostly populated with the "files and directories" lesson material available in: _includes/guide-shell/filedir

Normally, I would not be in favor of accepting duplicate material into the SWC repository. It is more work for everybody maintaining the lesson material to handle duplicate content, especially when it exists in different formats. It is very hard to automatically diff the lesson content, so changes to the material must be determined manually or from commit logs.

There are a couple of mitigating circumstances in this particular situation:

1. The filedir material had already been experimentally duplicated by @gvwilson into the instructor notebook when @ACharbonneau submitted the PR, her commits are not responsible for the duplication.
2. We are reorganizing lesson content (again). I am deferring responsibility for merging this material and the content in filedir to the organizers of the new material, @ethanwhite and @gvwilson. Any help from @ACharbonneau in summarizing how she improved the material will be appreciated by them. I've noticed a lot of reorganization and the introduction of concepts in a different order, but I didn't have the time to determine the complete set of changes.
3. We took a really long time to review this PR, and one of the mentors (especially me!) should have caught the duplication earlier.

Content and Naming

This notebook provides a complete guide to file directories in longform prose. Since the instructor's notebook seems to provide nothing else of substantial value, I've suggested removing it as well as renaming shell-overview.ipynb to shell-filedir.ipynb in commit: 138fe84.

Automated differencing between shell-filedir.ipynb and _includes/guide-shell/filedir

I tried using HTMLDiff to see if it would generate meaningful output:

Convert the notebook into HTML

ipython nbconvert shell-filedir.ipynb

Checkout the current working copy of the lesson:

git checkout origin/gh-pages ../../_includes/guide-shell/filedir/lesson.html

HTMLDiff:

htmldiff ../../_includes/guide-shell/filedir/lesson.html shell-filedir.html > lesson_diff.html

It didn't, and this hopefully drives home the point of why duplicating content in the repository is a bad idea. Amanda has made some great improvements to the material in the notebook. Unfortunately, there is no automated way for us to move or compare these improvements back to the original HTML source.

I'm doing a final technical/grammar review of the notebook, then I suggest we get this merged.

[ahmadia]

My revisions are in PR #153. I'd suggest somebody take a final review pass and merge them in. @ACharbonneau - sorry for the delays in reviewing this. A lot of this is due to my own confusion about how the material had changed. Hopefully it was worth the wait!

[ahmadia]

🎉