This repository has been archived by the owner on Dec 19, 2022. It is now read-only.
Some code fixes based on alerts from lgtm.com #21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://lgtm.com/projects/g/google/bottery/snapshot/84eb50a08253ebc2e531730521d1f27d9b9fb686/files/js/io.js#x1a7acd6235da44f4:1 ('loadMap' was defined twice with two different function signatures. I deleted the empty one)
|
yep, this looks good to me |
|
@calvinashmore: thanks for the quick merge! I'd love to hear your thoughts on lgtm's automated code reviews if you have a moment to try it 😄 |
|
It seems like a really helpful tool, if you've got your environment configured correctly for it. I tried setting up an account and analyzing a couple of my projects. It succeeded on the 10+ year old one but stumbled a couple of others due to build issues. It reminds me quite a bit of the automated tools we have at work. I think it would be especially effective if it were able to automate fixes (and maybe let a user interactively choose which to use). The tool might already do that but I haven't seen that particular behavior yet |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation About What Code Achieves:
Bottery is being analysed by lgtm.com: https://lgtm.com/projects/g/google/bottery/. This PR fixes four alerts (in separate commits) flagged up by lgtm.com. I hope it's of use!
Steps To Test:
The changes shouldn't change any of the behaviour of the code, exception for my change in
js/io.js. However, the code that I changed does not seem to be used.TODOs:
If you think these fixes are useful and would like to make sure problems like these are not accidentally introduced in the future: consider enabling automatic code reviews through PR integration on https://lgtm.com/projects/g/google/bottery/ci/
Here's an example of how NASA are using automatic code reviews for their Open-MBEE project: Open-MBEE/exec-cameo-mdk#105
(full disclosure: I'm part of the team behind lgtm.com 😄 — let me know if you have any feedback!)