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

test cases for JSON marshalling #1144

Merged
merged 10 commits into from
Apr 5, 2019

Conversation

vaibhavsingh97
Copy link
Contributor

Related Issue: #55

@googlebot googlebot added the cla: yes Indication that the PR author has signed a Google Contributor License Agreement. label Apr 2, 2019
@codecov
Copy link

codecov bot commented Apr 2, 2019

Codecov Report

Merging #1144 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1144   +/-   ##
=======================================
  Coverage   70.18%   70.18%           
=======================================
  Files          84       84           
  Lines        5792     5792           
=======================================
  Hits         4065     4065           
  Misses        947      947           
  Partials      780      780

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14c49e1...b45b1d2. Read the comment docs.

@vaibhavsingh97 vaibhavsingh97 changed the title pulls_comments_test.go: test case for JSON marshalling test cases for JSON marshalling Apr 2, 2019
@vaibhavsingh97
Copy link
Contributor Author

@gmlewis All test case passes, still travis failed 😞. Please let me know the possible fix

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 3, 2019

Without looking at the Travis CI logs, I can see that at least the first file has not been run through gofmt.

Please see CONTRIBUTING.md and follow the directions there. Thanks.

@vaibhavsingh97
Copy link
Contributor Author

vaibhavsingh97 commented Apr 3, 2019

@gmlewis I already ran go fmt ./... on my patch before sending PR

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 3, 2019

Looking at the logs from Travis CI, it says:

$ diff -u <(echo -n) <(gofmt -d -s .)
--- /dev/fd/63	2019-04-02 22:46:57.657608030 +0000
+++ /dev/fd/62	2019-04-02 22:46:57.657608030 +0000
@@ -0,0 +1,12 @@
+diff -u github/gists_test.go.orig github/gists_test.go
+--- github/gists_test.go.orig	2019-04-02 22:46:57.721579604 +0000
++++ github/gists_test.go	2019-04-02 22:46:57.721579604 +0000
+@@ -43,7 +43,7 @@
+ 			URL:         String("u"),
+ 		},
+ 		Files: map[GistFilename]GistFile{
+-			"gistfile.py": GistFile{
++			"gistfile.py": {
+ 				Size:     Int(167),
+ 				Filename: String("gistfile.py"),
+ 				Language: String("Python"),
The command "diff -u <(echo -n) <(gofmt -d -s .)" exited with 1.

What version of go are you using?
After running go fmt ./..., did you remember to commit the changes before pushing to your PR?
What does git status say in your repo?

@vaibhavsingh97
Copy link
Contributor Author

go version go1.11.5

image

I did commit the changes, before sending the PR. Should I retrigger travis, might be there was some issue?

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 3, 2019

No, there is no need, as it is obviously not formatted properly.
Click on the Files changed tab in GitHub and scroll down... you will see misalignments in many places.
Try pushing to the PR again with the correctly-formatted code.

@vaibhavsingh97
Copy link
Contributor Author

Gotcha! Will update after proper formatting the code 👍

github/gists_test.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thanks, @vaibhavsingh97!

LGTM.

Awaiting second LGTM before merging.

@gmlewis gmlewis requested a review from gauntface April 4, 2019 02:46
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.

thank you for the tests :)

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 5, 2019

Thank you, @gauntface!
Merging.

@gmlewis gmlewis merged commit d3f8bdf into google:master Apr 5, 2019
@vaibhavsingh97 vaibhavsingh97 deleted the patch/json-marshalling branch April 5, 2019 20:10
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.

4 participants