Removing render.RenderableNode #1204
Merged
Conversation
This file contains 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
paulbellamy
force-pushed
the
removing-renderablenode
branch
2 times, most recently
from
1eff227 to
8043001
Compare
|
squashed and rebased up to 8c0fddc |
paulbellamy
force-pushed
the
removing-renderablenode
branch
from
8043001 to
a6eff1d
Compare
| for _, k := range c.psMap.Keys() { | ||
| keys = append(keys, k) | ||
| func (c Counters) ForEach(f func(key string, val int)) { | ||
| if c.psMap != nil { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| @@ -121,7 +121,7 @@ var ( | |||
| // Node is arbitrary. We're free to put only precisely what we | |||
| // care to test into the fixture. Just be sure to include the bits | |||
| // that the mapping funcs extract :) | |||
| Client54001NodeID: report.MakeNode().WithLatests(map[string]string{ | |||
| Client54001NodeID: report.MakeNode().WithID(Client54001NodeID).WithLatests(map[string]string{ | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
Like this a lot. Some stuff in experimental needs fixing, and I think we need to add some testing for the final, fully rendered NodeSummaries. Also I think we need to address #836 ASAP. And we can potentially experiment with not merging in existing metadata when we do a derived node. |
|
Have run against example app, seems to do all the right things. |
paulbellamy
force-pushed
the
removing-renderablenode
branch
6 times, most recently
from
03d5b65 to
28f5957
Compare
|
LGTM on this. |
Squash of: - use detailed.Summaries to render topology nodes - ban merging nodes of different topologies (they should be mapped) - need to prune parents when mapping node types - render container images by id if they have no name - remove separate render ids and prune parents in NewDerived* - don't render metrics/metadata for groups of nodes - fixing up tests - removing pending unit tests (for mapping.go, for now) - updating experimental dir for RenderableNode removal
Squash of: - including children in topologies_test.go - report.Node.Prune should prune children also - rewrote ShortLivedInternetConnections test to express its intent - adding tests for detail Summary rendering
paulbellamy
force-pushed
the
removing-renderablenode
branch
from
28f5957 to
fe6203f
Compare
|
Rebased on 3744f47 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Includes and supersedes #1169. Will need a rebase on master.
TODO: