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

Use structs, for easier testing(?) #31

Merged
merged 1 commit into from
May 13, 2015

Conversation

simonjohansson
Copy link
Contributor

Want to run this trough you guys/gals out there. As pointed out by #17 we want more tests!

Instead of logging in each of the functions, create a struct instead. Then call AnnotateWithAppData() on the struct, then simply call .Log() on it.

This is a start to break out any external calls as much as possible from the logic to allow easier testing.

This also kinda addresses @mrdavidlaing comment over at #6

What do you think?

@simonjohansson
Copy link
Contributor Author

@shinji62 what do you think?

@shinji62
Copy link
Contributor

Seems ok, but you need to rebase first before merging :)
And maybe squash your commit

@simonjohansson
Copy link
Contributor Author

Yeah, ofc. This was just a throwaway branch for playing around anyways.

@simonjohansson simonjohansson force-pushed the UseStructsForLoggingEvents branch 3 times, most recently from 8a21ef6 to dfee94a Compare May 13, 2015 15:25
simonjohansson added a commit that referenced this pull request May 13, 2015
…ngEvents

Use structs, for easier testing(?)
@simonjohansson simonjohansson merged commit 7483118 into develop May 13, 2015
@simonjohansson simonjohansson deleted the UseStructsForLoggingEvents branch May 13, 2015 15:31
@simonjohansson simonjohansson restored the UseStructsForLoggingEvents branch June 25, 2015 14:44
@simonjohansson simonjohansson deleted the UseStructsForLoggingEvents branch June 25, 2015 14:46
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.

None yet

2 participants