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

Warning fixes and other cleanups #62

Merged
merged 4 commits into from
Jul 27, 2016
Merged

Warning fixes and other cleanups #62

merged 4 commits into from
Jul 27, 2016

Conversation

mvdan
Copy link
Contributor

@mvdan mvdan commented Jul 21, 2016

No description provided.

body, _ := ioutil.ReadFile(filename)
w.Write([]byte(body))
}

func TestNewClient(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Please do not delete these unused test functions. We need to start adding some tests and these helper functions will be needed when we do... So I would like to keep these in.

@svanharmelen
Copy link
Member

Hi @mvdan, looks good! Thanks!

I would like to keep the (currently) unused test helper functions, so if you can revert deleting them I'll merge the rest of this PR 👍

@svanharmelen
Copy link
Member

@mvdan I still would like to pull this one in, but you would first have to rebase it and revert the part where you delete the unused test functions. If you can do that, that would be great and I'll merge this PR right after... Thanks!

@mvdan
Copy link
Contributor Author

mvdan commented Jul 26, 2016

I just went on holidays right after opening this, so give me a couple of days :)

@svanharmelen
Copy link
Member

Sure, no rush! Thx 😀

It has methods to write strings and bytes already, which is much easier
and also faster than going through a regular write of []byte.
@mvdan
Copy link
Contributor Author

mvdan commented Jul 27, 2016

@svanharmelen done.

@svanharmelen svanharmelen merged commit 42a7783 into xanzy:master Jul 27, 2016
@mvdan mvdan deleted the warnings branch July 27, 2016 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants