Skip to content

engine/store: s/deployInfo/containerInfo, can attach multiple containerInfos to a BuildState if that target is deployed to multiple containers [ch3004]#1921

Merged
maiamcc merged 5 commits into
masterfrom
maiamcc/build-state-container-info
Jul 26, 2019
Merged

engine/store: s/deployInfo/containerInfo, can attach multiple containerInfos to a BuildState if that target is deployed to multiple containers [ch3004]#1921
maiamcc merged 5 commits into
masterfrom
maiamcc/build-state-container-info

Conversation

@maiamcc

@maiamcc maiamcc commented Jul 26, 2019

Copy link
Copy Markdown
Contributor

Hello @landism, @dbentley,

Please review the following commits I made in branch maiamcc/build-state-container-info:

2cba776 (2019-07-26 14:42:13 -0400)
tests

0d8c430 (2019-07-26 14:39:29 -0400)
make containerInfo for all containers on pod

ca0d844 (2019-07-26 12:42:36 -0400)
store multiple container infos (theoretically)

b6a00b1 (2019-07-26 11:48:18 -0400)
s/deployInfo/containerInfo

Code review reminders, by giving a LGTM you attest that:

  • Commits are adequately tested
  • Code is easy to understand and conforms to style guides
  • Incomplete code is marked with TODOs
  • Code is suitably instrumented with logging and metrics

@maiamcc
maiamcc requested review from dbentley and landism July 26, 2019 18:43

@dbentley dbentley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, but I may have missed something, cause I haven't been in here that deep recently...

Comment thread internal/store/build_result.go Outdated
// Check to see if there's a single, unambiguous Ready container
// in the given PodSet. If so, create a DeployInfo for that container.
func NewDeployInfo(iTarget model.ImageTarget, deployID model.DeployID, podSet PodSet) DeployInfo {
// IF all containers running the given image are ready, returns info for them.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/IF/If/?

@maiamcc
maiamcc merged commit d92acf8 into master Jul 26, 2019
@maiamcc
maiamcc deleted the maiamcc/build-state-container-info branch July 26, 2019 19:22
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.

2 participants