Skip to content

Conversation

gmlewis
Copy link
Collaborator

@gmlewis gmlewis commented Jun 14, 2019

After looking at the coverage details here: https://codecov.io/gh/google/go-github/tree/master/github
and noticing that the String methods account for a non-trivial percentage of uncovered code, I decided to write some.

Then, after writing a couple tests by hand, I realized that this is another great opportunity for go generate to write this mundane code for us and put it in a test file that nobody needs to care or worry about (github-stringify_test.go).

Getting these trivial functions out of the way, I believe that the coverage details will now have more meaning and we can more clearly identify and focus our attention on methods that need better test coverage.

@googlebot googlebot added the cla: yes Indication that the PR author has signed a Google Contributor License Agreement. label Jun 14, 2019
@gmlewis gmlewis requested a review from gauntface June 14, 2019 05:03
@gmlewis
Copy link
Collaborator Author

gmlewis commented Jun 19, 2019

friendly ping @gauntface

Copy link
Contributor

@gauntface gauntface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is seriously cool!!

Thank you for coming up with a generic and future proof way to improve the coverage.

var (
verbose = flag.Bool("v", false, "Print verbose log messages")

sourceTmpl = template.Must(template.New("source").Funcs(funcMap).Parse(source))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: could you move this to after the definition of funcMap.

@gmlewis
Copy link
Collaborator Author

gmlewis commented Jun 21, 2019

Thank you, @gauntface!
Merging.

@gmlewis gmlewis merged commit 0573e88 into google:master Jun 21, 2019
@gmlewis gmlewis deleted the gen-stringify-tests branch June 21, 2019 01:47
n1lesh pushed a commit to n1lesh/go-github that referenced this pull request Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indication that the PR author has signed a Google Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants