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

The required var has been added. #356

Merged
merged 2 commits into from
Oct 10, 2017
Merged

Conversation

varshav0119
Copy link
Contributor

I'm a first-timer and I think this is what you want?
Just adding a var to that line.

The var has been inserted in place.
Update LogToBugLogHQ.cfc with the required var
@atuttle
Copy link
Owner

atuttle commented Oct 10, 2017

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:

  • You should reference the original issue in the PR. Just mentioning it like "un-vared variable #353" will create a link from the issue to this PR, but GitHub supports an even more powerful feature sometimes called powerful commit messages (can use them in commit messages, PR title and body). Use text like "Resolves un-vared variable #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, if you'd like to check it out.

At any rate, this is great. Thanks so much! 👍

@atuttle atuttle merged commit 6de018b into atuttle:master Oct 10, 2017
@varshav0119
Copy link
Contributor Author

varshav0119 commented Oct 11, 2017 via email

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.

2 participants