Closed varshav0119 closed 7 years ago
Wow this and #357 came in just two minutes apart. I guess since yours came first, I'll merge this one. So congrats on your first pull request, @FruitVodka! (And honorable mention to @vaani98)
This is indeed the code fix that was requested. Thanks!
Some advice for future PRs:
At any rate, this is great. Thanks so much! 👍
Thanks a lot for merging my request and the tips!! I will definitely keep those in mind for the next time :)
On 10 Oct 2017 11:52 p.m., "Adam Tuttle" notifications@github.com wrote:
Wow this and #357 https://github.com/atuttle/Taffy/pull/357 came in just two minutes apart. I guess since yours came first, I'll merge this one. So congrats on your first pull request, @FruitVodka https://github.com/fruitvodka! (And honorable mention to @vaani98 https://github.com/vaani98)
This is indeed the code fix that was requested. Thanks!
Some advice for future PRs:
- You should reference the original issue in the PR. Just mentioning it like "#353 https://github.com/atuttle/Taffy/issues/353" will create a link from the issue to this PR, but GitHub supports an even more powerful feature sometimes called powerful commit messages https://help.github.com/articles/closing-issues-using-keywords/ (can use them in commit messages, PR title and body). Use text like "Resolves
353 https://github.com/atuttle/Taffy/issues/353" and when the PR
gets merged GitHub will automatically close that issue. Pretty awesome. 😎
- You're fine this time, but in a project where you expect to make several contributions it is generally a best practice to put each contribution into its own branch. I cover why and how in a good amount of detail in this blog post http://adamtuttle.codes/your-first-github-pull-request/, if you'd like to check it out.
At any rate, this is great. Thanks so much! 👍
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/atuttle/Taffy/pull/356#issuecomment-335563527, or mute the thread https://github.com/notifications/unsubscribe-auth/AeTEJACbX6ZrPncEXVon3-JuJAo6V6Zfks5sq7XggaJpZM4P0RUX .
I'm a first-timer and I think this is what you want? Just adding a var to that line.