Closed leemc-data-ed closed 2 years ago
I made most of the changes we talked through in our meeting in terms of the content and most formatting. I think the only outstanding things now are to figure out how to best resize images.
This is looking great! There are just a couple of formatting things and a couple of content things, but I think it's really close.
For the formatting stuff, I think it's just liascript being fussy-- I'm happy to just fix it if that's okay with you. Some of the quizzes still aren't displaying correctly, and using certain symbols seems to mess up the formatting.
For the content:
And then I think that the "Proceed with caution" warning you have on line 207, in the Permissions section, would be perfect to go into an "Important" highlight box. To do that, you use
Thanks @leemc-data-ed I am totally comfortable with you making those formatting changes and I will let you know once I've added those content and accessibility changes.
@leemc-data-ed incorporated some more changes, grateful as always for your feedback!
Your changes look great! I made the quizzes so that the explanations come up after the learner submits the answer, fixed the weirdness with the symbols (I can't figure out why it's happening, but surrounding them in backticks fixes it), and adjusted the widths of the images on in the Commands page so that they're the same size. Check it out and let me know what you think!
looks great, thanks so much for working through the technical issues!
should I close this issue and merge the branch?
We have to get the PR approved before we can merge, I want one more set of eyes too look over everything and make sure there isn't anything we missed, then we can approve and merge!
Hi! I looked over this module as well, since Meredith requested some fresh eyes :) Overall I really like it! It does an excellent job of staying concise and approachable while still covering quite a lot of ground, which is a hard balance to strike. Really awesome work here, and thank you to both of you for doing this! I'm on the fence about whether or not to just approve and publish as-is, because while I think it's probably good enough already, I do see a couple opportunities to make it stronger. I'm happy to leave that call up to you, though: If you're ready to just be done, you have my blessing! If you have a little steam left in you for more edits, though, here are my suggestions:
In addition to my earlier offer to just let it be done and approve as-is (which is genuine! I think it's a very solid module already.), I also want to mention that, if you want to pursue some of these changes, I would be more than happy to offer more concrete edit suggestions, i.e. push a commit with some changes you could use or not. I know it can be exhausting to interpret other people's suggestions for edits and generate new content vs. just evaluating some concrete edits provided as suggestions, and I would be very happy to share the lift on this! :) Just let me know.
Thank you for your thoughtful and well-explained feedback @rosemm. I tried incorporating some of your changes but I'd be grateful for more explicit help in making the distinction between shell and kernel because I think it is an important one but I am having trouble weaving it in elegantly. Thank you again!
Sure thing! I ran into the same problem you did trying to figure out where to squeeze it in, so I tried doing a slight rearrange of the structure in that first section to accommodate it. Basically, I broke your list into two parts (the bits that answer "what is shell scripting" vs the bits that answer "when would I use it"), and then popped in a "learnmore" box about the term "shell" at the end of the first part, since that seemed like the right moment for it. Here's the commit: c97afd3 Feel free to take what works for you and leave what doesn't, of course! P.S. When I opened the markdown file I did catch two little formatty things I hadn't seen when I was reviewing the module earlier: Ideally there should be a period at the end of each image's alt text, so I put those in, and the content you had under "is this module right for me" in the overview should be saved in the YAML as long_description, so I moved it up there. I put those changes in separate commits, to keep things tidy, so you'll see a string of three commits from me today.
@rosemm looks great thank you, I really appreciate you weaving everything together!
@nafeldman so I needed to go back in and fix some formatting weirdness with the quizzes-- now the extra explanation text will only come up once the learner has selected the correct answer.
I also made some minor tweaks to the language in a couple of places, to try to avoid using words like "easy" or other potentially minimizing words as Rose suggested and to clarify things in a couple of places. Take a look and let me know if I missed the mark on anything! Otherwise, I think we're good, and once I get your final okay, I'll approve and merge :)
Thanks again for all your awesome work on this!
@leemc-data-ed LGTM! feel free to merge :)
Merged with main, closing this issue!
Module Quality Assurance Report for PR #34
Date: 2022-01-03 Reviewer: Meredith Lee Name of Module: Command Line 101 Current Liascript URL: https://liascript.io/course/?https://raw.githubusercontent.com/arcus/education_modules/bash-scripting-101/bash_scripting_101/bash_scripting_101.md#1 Current Version of Module (use the latest commit value): a2689bc
Checklist Reports:
Structural Elements
Content
Organization
Formative assessment
Videos and images
Branch References to Change prior to PR
List here any internal references (stated or hyperlinked) that work now because they refer to the named branch, but will not work once this is on the main branch and the named branch is deleted.