Closed catcherwong closed 7 years ago
Looks great!
Also you could have just force pushed to your first PR, though this is perfectly fine too. Git can be tricky sometimes
As @yahehe said, force pushing (git push -f
) is good for this situation because it allows us to track your progress in a single PR! And since it's only pushing to your repo and not this one, there's no (or minimal) risk. Never force push to a master branch though.
And the PR looks good. Thanks for making the changes! Update the name of that one test and I'll merge it in.
@yahehe @jnallard Thanks for your explanation ! 😄 I'll pay attention to that next time !
Do you know how to amend or squash commits? It would be great if you could add your rename change into one of your other commits (because 'rename' is not a very helpful commit name). I'd recommend trying either squashing or amending! The first two stack overflow answers show you how to do both: https://stackoverflow.com/questions/2563632/how-can-i-merge-two-commits-into-one
I'd recommend copying your branch locally first, just because it's easy to mess up when you're learning how to do these commands. (And I don't care too much about the 'rename' commit, but I want to help you with your git skills!) If you need help, ask away!
@jnallard Thank you vary much ! I have amended it .
To be honest , I didn't amend or squash commits before . Thanks for your patient explanation again !
First of all , I feel so sorry for my mistakes on PR #140 . :pensive::pensive:
And here is somethings I have done for this PR