-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[BEAM-121] Additional APIs for registering DisplayData #145
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 based on feeback from #123, #124, #125, #126. I will update those PRs to consume these APIs once merged. |
| return builder.toString(); | ||
| } | ||
|
|
||
| private static String convertNamespace(Class<?> nsClass) { |
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.
namespaceOf(...)? classToNamespace? (just thinking out loud, feel free to ignore if you don't think they're improvements).
|
LGTM Only thing that occurred to me is that we could reduce the number of methods on the builder by putting more logic into the
Then the builder API just needs a method to add an entry if its not null:
This is also nice when including the inference:
|
|
Backported via GoogleCloudPlatform/DataflowJavaSDK#216 |
[euphoria-spark] Propagate exception in SparkFlowTranslator
* feat: integrate limit_to_last changes from apache#57 to async * fix: whitespace in docs * fix: whitespace in docs
fix BoundValue cast issue
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).
number, if there is one.
Individual Contributor License Agreement.