[BEAM-231] Remove ClassForDisplay helper type #259
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
R: @bjchambers |
| super.populateDisplayData(builder); | ||
|
|
||
| Combine.populateDisplayData(builder, fn, fnClass); | ||
| Combine.populateDisplayData(builder, fn, fnDisplayData); |
Contributor
There was a problem hiding this comment.
Can you add comments here about the choice of calling populateDisplayData vs. builder.include(...)?
Contributor
Author
There was a problem hiding this comment.
I've added usage comments on DisplayData.Builder#include and HasDisplayData#populateDisplayData-- let me know what you think.
Contributor
|
In |
Contributor
Author
|
I've addressed all feedback so far. Please take another look. @bjchambers |
Contributor
Author
|
travis failure looks like a flake. I believe this is ready to go. |
| * <p>Including subcomponent display data via | ||
| * {@code DisplayData.Builder.include(HasDisplayData)} ensures that the | ||
| * {@link Item#getNamespace() namespace} of registered {@link Item display items} | ||
| * is properly set to the namespace of the subcomponent. To register display data from a base |
Contributor
There was a problem hiding this comment.
Using {@code include(subcomponent)} will associate each of the registered items with the namespace of the {@code subcomponent} being registered. To register display data in the current namespace use {@code builder.include(subcomponent)} instead.
@see ...
Contributor
|
Minor wording comments, then LGTM |
swegner
force-pushed
the
classfordisplay
branch
from
c733ae2 to
9bbc143
Compare
Contributor
Author
|
I've addressed all feedback so far. Please take another look. @bjchambers |
| * Using {@code include(subcomponent)} will associate each of the registered items with the | ||
| * namespace of the {@code subcomponent} being registered. To register display data in the | ||
| * current namespace, such as from a base class implementation, use | ||
| * {@code super.populateDisplayData(builder)} instead. |
Contributor
There was a problem hiding this comment.
{@code include(subcomponent)} -> {@code builder.include(subcomponent)}
{@code super.populateDisplaydtaa(builder)} -> {@code subcomponent.populateDisplayData(builder)}
That way we're consistent with the example and suggestion.
Contributor
Author
|
I've addressed all feedback so far. Please take another look. @bjchambers |
Contributor
|
LGTM |
Contributor
Author
|
Backport: GoogleCloudPlatform/DataflowJavaSDK#230 |
iemejia
pushed a commit
to iemejia/beam
that referenced
this pull request
Jan 12, 2018
mareksimunek
added a commit
to mareksimunek/beam
that referenced
this pull request
May 9, 2018
…ribe OutputDataset
mareksimunek
added a commit
to mareksimunek/beam
that referenced
this pull request
May 9, 2018
fixed tests [euphoria-core] hints in getBasicOps are only on last operator [euphoria-core] code style corrections [euphoria-core] code style corrections 2
mareksimunek
pushed a commit
to mareksimunek/beam
that referenced
this pull request
May 9, 2018
[euphoria-core] apache#259 Hints are not runtime specific.
dmvk
pushed a commit
to dmvk/beam
that referenced
this pull request
May 15, 2018
…ribe OutputDataset
dmvk
pushed a commit
to dmvk/beam
that referenced
this pull request
May 15, 2018
fixed tests [euphoria-core] hints in getBasicOps are only on last operator [euphoria-core] code style corrections [euphoria-core] code style corrections 2
mareksimunek
added a commit
to seznam/beam
that referenced
this pull request
Jul 9, 2018
…ribe OutputDataset
mareksimunek
added a commit
to seznam/beam
that referenced
this pull request
Jul 9, 2018
fixed tests [euphoria-core] hints in getBasicOps are only on last operator [euphoria-core] code style corrections [euphoria-core] code style corrections 2
dmvk
pushed a commit
to seznam/beam
that referenced
this pull request
Aug 17, 2018
…ribe OutputDataset
dmvk
pushed a commit
to seznam/beam
that referenced
this pull request
Aug 17, 2018
fixed tests [euphoria-core] hints in getBasicOps are only on last operator [euphoria-core] code style corrections [euphoria-core] code style corrections 2
dmvk
pushed a commit
to seznam/beam
that referenced
this pull request
Oct 5, 2018
…ribe OutputDataset
dmvk
pushed a commit
to seznam/beam
that referenced
this pull request
Oct 5, 2018
fixed tests [euphoria-core] hints in getBasicOps are only on last operator [euphoria-core] code style corrections [euphoria-core] code style corrections 2
damccorm
added a commit
that referenced
this pull request
Dec 9, 2022
…f-hosted runners (#23134) * Updating build_playground_backend workflow (#167) Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local> * Added master changes in build_playground_backend to avoid merge conflicts * Reverted GO_VERSION and BEAM_VERSION to have the same as master build_playground_backend * Switching trigger to pull_request (#259) * Switching to pull_request * Removing ref from checkout Co-authored-by: Elias Segundo Antonio <eliassegundo.segundo@gmail.com> Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local> Co-authored-by: elink22 <103056145+elink22@users.noreply.github.com> Co-authored-by: Danny McCormick <dannymccormick@google.com>
lostluck
pushed a commit
to lostluck/beam
that referenced
this pull request
Dec 22, 2022
…f-hosted runners (apache#23134) * Updating build_playground_backend workflow (apache#167) Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local> * Added master changes in build_playground_backend to avoid merge conflicts * Reverted GO_VERSION and BEAM_VERSION to have the same as master build_playground_backend * Switching trigger to pull_request (apache#259) * Switching to pull_request * Removing ref from checkout Co-authored-by: Elias Segundo Antonio <eliassegundo.segundo@gmail.com> Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local> Co-authored-by: elink22 <103056145+elink22@users.noreply.github.com> Co-authored-by: Danny McCormick <dannymccormick@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.