Skip to content

Conversation

@samirahekmati
Copy link

@samirahekmati samirahekmati commented Mar 5, 2025

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

@samirahekmati samirahekmati added 📅 Sprint 1 Assigned during Sprint 1 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 5, 2025
Copy link

@ehwus ehwus left a comment

Choose a reason for hiding this comment

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

This is great work, you've demonstrated a really good understanding of the shell tools and bash scripting in general that will be a solid base for the remainder of the course and your career. There are some issues that I've raised with the way that the PR was presented, but those are all largely to do with attention to detail in pull requests, and it's very clear that you have understood the material well.


awk '/London/ {print $1, $NF}' scores-table.txt

awk '{if($2 == "London")print $1, $NF}' scores-table.txt
Copy link

Choose a reason for hiding this comment

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

issue: It's great to see two solutions here, which one do you think is better?

I think there's a lot of value in showing both, but please make sure to show this in a comment rather than being a part of the script, as we're essentially running it two times which is not the expected output:

Screenshot 2025-03-16 at 08 02 22

# TODO: Write a command to output the total of adding together all players' first scores.
# Your output should be exactly the number 54.

awk '{sum += $3} END {print sum}' scores-table.txt No newline at end of file
Copy link

Choose a reason for hiding this comment

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

praise: great to see you hit the strech goals here!

# Your output should contain 6 lines, each with one word and one number on it.
# The first line should be "Ahmed 15". The second line should be "Basia 37"

awk '{sum=0; for (i=3; i<=NF; i++) sum+=$i; print $1, sum}' scores-table.txt No newline at end of file
Copy link

Choose a reason for hiding this comment

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

praise: great demonstration of performing a sum in awk here!




# method 1 (relative path)
Copy link

Choose a reason for hiding this comment

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

issue: Once again, let's think carefully about executing different methods multiple times

We also always need to consider the portability of our code. Always remember that someone else will be running code that you submit for a PR, why am I getting this error?

Screenshot 2025-03-16 at 08 05 18

# I was tempted to take a bite of it.
# But this seemed like a bad idea...

echo hello world
Copy link

Choose a reason for hiding this comment

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

nitpick: Lines like this are great for debugging and setting things up / learning, but lets make sure that we're removing them before submitting work for review.


echo hello world

cat ../helper-files/*.txt No newline at end of file
Copy link

Choose a reason for hiding this comment

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

praise: good use of the wildcard here, and the extension is a clever way to make sure that only text files would ever be read


cat ../helper-files/*.txt | cat -n

# cat ../helper-files/*.txt first combines all files into a single stream.
Copy link

Choose a reason for hiding this comment

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

praise: comments like this are great for showing your working! I think following this pattern in other cases where you've left duplicate code uncommented in the script would be great.

# The output should be a list of names: helper-1.txt, helper-2.txt, helper-3.txt.


cd child-directory
Copy link

Choose a reason for hiding this comment

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

question: This definitely works, but can we do it without changing directory?


echo "Move the starting number to the end of the line:"

sed 's/^\([0-9]*\) \(.*\)$/\2 \1/' input.txt No newline at end of file
Copy link

Choose a reason for hiding this comment

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

praise: great regular expression usage here!

@ehwus ehwus added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed Volunteer to add when completing a review with trainee action still to take. 📅 Sprint 1 Assigned during Sprint 1 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants