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

Add MarshalJSON function to raw.BSON #108

Closed
wants to merge 1 commit into from

Conversation

SteelPhase
Copy link

This resolves #107 using the example provided in the issue.

@domodwyer
Copy link

Hi @SteelPhase

I'm up for this - I can see how it might help the users of bson.Raw building APIs etc and it's totally opt-in - other users don't pay a performance overhead for this added functionality - good idea!

We're pretty strict on tests for maintainability though, would you mind adding a couple for this? A happy-path test and a couple edge cases, preferably table driven?

Thanks for the PR!
Dom

@SteelPhase
Copy link
Author

Yeah. i'll look into adding tests.

@domodwyer
Copy link

Hi @SteelPhase

I'm going to close this because it's been a couple months - we'll still happily accept it if you have the time to add tests though.

Dom

@domodwyer domodwyer closed this Jun 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants