-
Notifications
You must be signed in to change notification settings - Fork 378
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
Fixing this code be compatiable with v2.x I also added some comments … #676
Conversation
…and moved variable declarations to the top to clean up the code a bit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for sending a PR! I've left some feedback for some small changes
Codecov Report
@@ Coverage Diff @@
## main #676 +/- ##
=======================================
Coverage 84.41% 84.41%
=======================================
Files 8 8
Lines 693 693
Branches 206 206
=======================================
Hits 585 585
Misses 71 71
Partials 37 37 Continue to review full report at Codecov.
|
@stevengill I created another pull request with your suggested updates to my code. |
@stevengill all changes suggested have been committed |
Looks good to me. @stevengill can you take a look at this again? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…and moved variable declarations to the top to clean up the code a bit
Summary
Describe the goal of this PR. Mention any related Issue numbers.
Requirements (place an
x
in each[ ]
)