Skip to content

GROOVY-7946 - allow writable values for StreamingJsonBuilder#429

Closed
graemerocher wants to merge 5 commits intoapache:GROOVY_2_4_Xfrom
graemerocher:json-builder-writable-improvement
Closed

GROOVY-7946 - allow writable values for StreamingJsonBuilder#429
graemerocher wants to merge 5 commits intoapache:GROOVY_2_4_Xfrom
graemerocher:json-builder-writable-improvement

Conversation

@graemerocher
Copy link
Copy Markdown
Contributor

@shils
Copy link
Copy Markdown
Member

shils commented Sep 24, 2016

+1

* Writes the given Writable as the value of the given attribute name
*
* @param name The attribute name The attribute name
* @param json The value The writable
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

descriptions for the params seems to be repeated twice.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice catch. Fixed.

@jwagenleitner
Copy link
Copy Markdown
Contributor

+1

@graemerocher
Copy link
Copy Markdown
Contributor Author

Any chance this can be merged? Thanks...

@graemerocher
Copy link
Copy Markdown
Contributor Author

Actually hold off merging, found an a potential issue and will revise the test case and update the pull request

@graemerocher
Copy link
Copy Markdown
Contributor Author

Revised pull request, since it created issues handling GString instances which are also Writable

@graemerocher
Copy link
Copy Markdown
Contributor Author

The failures on Travis seem to be unrelated network issues?

@jwagenleitner
Copy link
Copy Markdown
Contributor

PR merged, thanks! Travis doesn't seem to always like building against the 2_4_X branch, seems to fail spuriously. Hope you don't mind but I squashed the commits.

blindpirate pushed a commit to blindpirate/groovy that referenced this pull request Nov 11, 2016
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.

3 participants