WestMidlands | 25-SDC-Nov | Sara Tahir | Sprint 1 | Individual Shell Tools#201
WestMidlands | 25-SDC-Nov | Sara Tahir | Sprint 1 | Individual Shell Tools#201SaraTahir28 wants to merge 7 commits intoCodeYourFuture:mainfrom
Conversation
SaraTahir28
added
the
Needs Review
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
6 similar comments
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good start on this sprint's tasks, I have spotted a few areas where you could improve code further
There was a problem hiding this comment.
You've comitted this by accident, can you revert it from this PR?
There was a problem hiding this comment.
Done, A great learning experience happened doing this :)
There was a problem hiding this comment.
Also where I can improve my code further, can you please share. Thanks
LonMcGregor
added
Reviewed
SaraTahir28
force-pushed
the
Feature/Individual-Shell-tools
branch
from
3a23ee7 to
665b5d9
Compare
SaraTahir28
added
the
Needs Review
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Thanks for removing the unneeded files. I can see one file you could improve further, see my comment below
| # 3 It looked delicious. | ||
| # 4 I was tempted to take a bite of it. | ||
| # 5 But this seemed like a bad idea... | ||
| cat ../helper-files/*.txt | cat -n No newline at end of file |
There was a problem hiding this comment.
You could probably solve this without piping to cat a second time. Can you figure out how?
There was a problem hiding this comment.
yes I could by passing a -n flag to first cat, but cat behaves differently on Mac OS (BSD cat) if I just pass a -n flag, My output gets numbered 1 at start of each file
1 Once upon a time...
1 There was a house made of gingerbread.
1 It looked delicious.
2 I was tempted to take a bite of it.
3 But this seemed like a bad idea...
which is not expected behavior I believe.
so in my command, first cat concatenates all the files into a single stream of text and second cat -n sees one continuous stream, not separate files so it numbers them correctly without resetting.
I have added a second command to my file and committed it too.
Thanks
There was a problem hiding this comment.
This sounds like a reasonable approach to work around the counting issue on mac, good idea!
LonMcGregor
removed
the
Needs Review
SaraTahir28
added
the
Needs Review
LonMcGregor
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR
I have answered all the questions for the Backlog
Questions
I have no Questions.Thankyou