Fix a few alerts from LGTM.com#35
Closed
robertbrignull wants to merge 9 commits intohatnote:masterfrom
robertbrignull:lgtm_alerts
Closed
Fix a few alerts from LGTM.com#35robertbrignull wants to merge 9 commits intohatnote:masterfrom robertbrignull:lgtm_alerts
robertbrignull wants to merge 9 commits intohatnote:masterfrom
robertbrignull:lgtm_alerts
Conversation
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR fixes all the alerts from LGTM.com that I thought were correct and useful. There's nothing massively critical but they are still results where it's nice to fix them and make the code a little cleaner. The rest of the results that I haven't addressed are either results in libraries or deliberate actions so I didn't want to change anything.
Let me know if any of the results don't make sense or if you think something is wrong. There are links on LGTM to the alert help which describes what the result means and why it is good to address it. For full disclosure I do work at the company that makes LGTM.com and I'm happy to answer any questions.
I've also added a
.lgtm.ymlfile to the repository which is what we use to allow you to customize the results. In this case it marks a few files as library code so you won't see the alerts in them. If you don't want this file then I'm happy to remove it from the PR, but hopefully it's not too intrusive.If you wanted then you could even enable automatic code review integration and then LGTM will analyze pull requests that get made and make sure that no problems creep back in. Entirely up to you.