-
-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add exercises for individual shell tools and shell pipelines #1
base: main
Are you sure you want to change the base?
Conversation
047b001
to
be397fb
Compare
be397fb
to
7fbe81a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've only done the individual-shell-tools
so far - since I wanted to get the feedback quicker and that's the only bit that's been merged on the curriculum - but it's looking good! Left a few comments about typos and minor suggestions.
Co-authored-by: Alasdair Smith <alasdairsmith100@gmail.com>
Co-authored-by: Alasdair Smith <alasdairsmith100@gmail.com>
Co-authored-by: Alasdair Smith <alasdairsmith100@gmail.com>
Co-authored-by: Alasdair Smith <alasdairsmith100@gmail.com>
Co-authored-by: Alasdair Smith <alasdairsmith100@gmail.com>
Co-authored-by: Alasdair Smith <alasdairsmith100@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Oh wait, sorry I forgot about the other commands - will review those now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple more comments/suggestions about the jq
& shell-pipelines
exercises, but looking pretty good!
jq/script-02.sh
Outdated
# TODO: Write a command to output the name of the person, then a comma, then their profession. | ||
# Your output should be exactly the string "Selma, Software Engineer", but should not contain any quote characters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one feels like a bit of an outlier in terms of difficulty.
I came up with jq -r '[ .name, .profession ] | join(", ")'
fwiw. But I see you used escapes within a string.
I think I see what you're going for with multiple selects but I wonder if at this stage it would be best to build an array instead of a string? That would avoid the need for escapes or join functions.
However, I do see that we're using join
in the immediate next exercise - and that doesn't feel too far outside to difficulty curve for me - so perhaps this is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I swapped the order of this and the next one (which is a "pipe to join
" exercuse), and added your join
as an alternate sample solution, WDYT?
Co-authored-by: Alasdair Smith <alasdairsmith100@gmail.com>
Achieving these exercises meets all of the shell tools learning objectives of the first two sprints.
Sample solutions can be seen in the HEAD commit of https://github.com/CodeYourFuture/Module-Tools/tree/individual-shell-tools-solutions (currently d827a5e)