-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[BEAM-199] Update DisplayData APIs to make overrides more understandable #247
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
Conversation
|
R: @bjchambers This PR is rather lengthy as it updates all
|
| /** | ||
| * Utility to append optional fields to display data, or register additional display data | ||
| * items. | ||
| * A display data item. DisplayData items are registered via {@link DisplayData.Builder#add} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A display data item. {@link Item} items are registered via {...}
3303b25 to
19665c8
Compare
|
I've addressed all feedback so far. Please take another look. @bjchambers |
| */ | ||
| public static DisplayData from(HasDisplayData component) { | ||
| checkNotNull(component); | ||
| checkNotNull(component, "Input component cannot be null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"component argument cannot be null"
|
|
||
| return false; | ||
| /** @see #withNamespace(Class) */ | ||
| public Item<T> withNamespace(String namespace) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be private?
|
A few nits, then LGTM. |
|
I've addressed all feedback so far. Please take another look. @bjchambers |
|
LGTM |
|
Backport: GoogleCloudPlatform/DataflowJavaSDK#230 |
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.