Skip to content
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

added a heelo mesage and a gif of a typing cat #145

Merged
merged 10 commits into from
Oct 29, 2018

Conversation

kt-lmb
Copy link
Contributor

@kt-lmb kt-lmb commented Oct 29, 2018

No description provided.

Copy link
Member

@yevhenorlov yevhenorlov left a comment

Choose a reason for hiding this comment

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

hi @kt-lmb, welcome to the course!

please remove empty lines 495-497, they are unnecessary.

README.md Show resolved Hide resolved
Copy link
Member

@OleksiyRudenko OleksiyRudenko left a comment

Choose a reason for hiding this comment

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

@kt-lmb, well done! But could you please place your image under gif/ directory?
You will probably need to rename it and change a reference to it in README.md

@OleksiyRudenko
Copy link
Member

OleksiyRudenko commented Oct 29, 2018

@kt-lmb
Meanwhile the repo's been updated. Please, resolve the conflict.
If you need any help, please, let us know.

Copy link
Member

@OleksiyRudenko OleksiyRudenko left a comment

Choose a reason for hiding this comment

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

@kt-lmb , not bad.
But it seems like you replaced someone's else code. Let's not make them upset.
Please, bring their code back.

You have also replaced a link to a local image with a link to a remote resource. This will definitely work.
Now the repo doesn't need neither ./giphy.gif nor ./gif/typingCat.gif as they aren't referred to anywhere. Please, remove those.

Copy link
Member

@yevhenorlov yevhenorlov left a comment

Choose a reason for hiding this comment

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

LGTM (Looks good to me)
good job!

@OleksiyRudenko
Copy link
Member

OleksiyRudenko commented Oct 29, 2018

@kt-lmb , you are almost done!
Remove images from your local repo (hint: use git rm <filename> <path/filename>)
Then commit and push.
A tiny step and you're on the top!

P.S. It might be challenging but the benefit for you is that you level-up your git skills!
I am eager to merge your PR 😉 Pls, mention me in the comment as soon as you feel ready or need help.

Hurry! There are other PRs that may provoke a merge conflict!

@OleksiyRudenko
Copy link
Member

@kt-lmb , you have successfully removed ./gif/typingCat.gif and also tried to remove ./gif/giphy.gif. But the latter has no relative path. Just do git rm giphy.gif as it is located in the root of your project.

Now there is also a formal merge conflict as README.md has been changed.

Will you resolve it? You need to keep only one of two alternatives (hint: the second one is empty but it is in place. Sounds weird, I know). Built-in github conflict resolver is very helpful.

@OleksiyRudenko
Copy link
Member

@kt-lmb grats! You did it!

@OleksiyRudenko OleksiyRudenko merged commit 936e120 into kottans:master Oct 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants