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

Go Report Card gave us a D :( #190

Closed
mbernier opened this issue Oct 21, 2017 · 6 comments
Closed

Go Report Card gave us a D :( #190

mbernier opened this issue Oct 21, 2017 · 6 comments
Labels
difficulty: very hard fix is very hard in difficulty

Comments

@mbernier
Copy link
Contributor

mbernier commented Oct 21, 2017

Issue Summary

We have some issues as reported by our nifty new Go Report Card badge in the readme. Can you fix these issues up?

Steps to Reproduce

  1. Go to the link above, see the sadness

Technical details:

  • sendgrid-go Version: master
  • Go Version: 1.5.1

It's hacktoberfest and we're giving away a shirt for any accepted PRs during October! https://dx.sendgrid.com/hacktoberfest

Note: Please do not fix the error reported by "Misspell" if that effects our grade, we are OK with it. :)

#121

@mbernier mbernier added difficulty: hard fix is hard in difficulty hacktoberfest difficulty: very hard fix is very hard in difficulty and removed difficulty: hard fix is hard in difficulty labels Oct 21, 2017
@srini156 srini156 mentioned this issue Oct 22, 2017
@mbernier
Copy link
Contributor Author

Looks like #121 does some of this, then #148 potentially as well.

@tariq1890
Copy link
Contributor

Hi @mbernier, #148 does fix it. However, the GoDoc messages could be more descriptive. If we could reach out to the developer who provided that PR, we could ask him to improve the GoDoc comments.

As of now, No PR fixes the gocyclo issues. I can take that up. To fix it, I can write a helper assert methods to assert the values in test cases. This way we don't have a high number statements just to assert every value. To fix the other gocyclo issue, we would need to refactor the inbound helper method. I can take a stab at this and provide extra test cases to ensure that the logic isn't changed in the process.

@srini156
Copy link
Contributor

@mbernier @tariq1890 - I have created merge request #196 for fixing the gocyclo issue. Please let me know your comments.

@Cleanse
Copy link
Contributor

Cleanse commented Oct 22, 2017

Will finish the golint issues for you here after dinner.

@Cleanse
Copy link
Contributor

Cleanse commented Oct 23, 2017

#200

@zealsprince
Copy link
Contributor

#224: Made a PR which fixes spelling as shown to be an issue on the Go Report Card. Also modified some formatting inconsistencies in a few of the comments in the same file. Should be an easy merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: very hard fix is very hard in difficulty
Projects
None yet
Development

No branches or pull requests

5 participants