Skip to content

Conversation

@swegner
Copy link
Contributor

@swegner swegner commented Apr 11, 2016

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify. (Even better, enable
    Travis-CI on your fork and ensure the whole test matrix passes).
  • Replace "<Jira issue #>" in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

@swegner
Copy link
Contributor Author

swegner commented Apr 11, 2016

R: @bjchambers

@bjchambers
Copy link
Contributor

I see test failures. Can you verify that nothing is broken?

}

addList(getProperties(), name, serializedItems);
List list = MAPPER.convertValue(displayData, List.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

List<?>

@swegner swegner force-pushed the displaydata-tweaks branch from 6701f4b to a15ed66 Compare April 12, 2016 17:16
@swegner
Copy link
Contributor Author

swegner commented Apr 12, 2016

Ok, this should build clean now. @bjchambers, can you take another look? Once this is in I'll rebase other PR's on top and we can have another round of feedback on those.


JsonNode json = MAPPER.readTree(MAPPER.writeValueAsBytes(data));
assertThat(json, hasItem(expectedJson(component.getClass(), "STRING", "string", "\"foobar\"")));
assertThat(json, hasItem(expectedJson(component.getClass(), "INTEGER", "long", "1234")));
Copy link
Contributor

Choose a reason for hiding this comment

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

This test may be easier to read (and verify it matches expectations) if it just has the expected value as a single string, or a string for each value.

@swegner
Copy link
Contributor Author

swegner commented Apr 13, 2016

I've addressed all feedback so far. Please take another look. @bjchambers

@bjchambers
Copy link
Contributor

LGTM

@asfgit asfgit closed this in 1915503 Apr 13, 2016
@swegner swegner deleted the displaydata-tweaks branch April 14, 2016 15:15
@davorbonaci
Copy link
Member

@swegner, did we backport this? (Cc: @bjchambers)

@swegner
Copy link
Contributor Author

swegner commented Apr 15, 2016

I haven't done any backporting work for recent PRs-- I'm not quite familiar with the process. I'll work with @bjchambers on this.

swegner pushed a commit to swegner/beam that referenced this pull request Apr 22, 2016
@swegner
Copy link
Contributor Author

swegner commented May 16, 2016

Backported via GoogleCloudPlatform/DataflowJavaSDK#216

iemejia pushed a commit to iemejia/beam that referenced this pull request Jan 12, 2018
mareksimunek pushed a commit to mareksimunek/beam that referenced this pull request May 9, 2018
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