Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Get container info from FactoryOS registry instead of cmdiag; Create service events for container status #20

Merged
merged 5 commits into from
Sep 23, 2020
Merged

Get container info from FactoryOS registry instead of cmdiag; Create service events for container status #20

merged 5 commits into from
Sep 23, 2020

Conversation

spaceisfun
Copy link
Member

No description provided.

@spaceisfun spaceisfun added the enhancement New feature or request label Sep 19, 2020
@spaceisfun spaceisfun requested a review from a team September 19, 2020 22:36
@spaceisfun spaceisfun self-assigned this Sep 19, 2020
@spaceisfun spaceisfun linked an issue Sep 19, 2020 that may be closed by this pull request
Container logic should not depend on parsing console output #19
Closed
@spaceisfun
Copy link
Member Author

@microsoft/onecore any chance of a review by EOD here? Thanks!

hajya
hajya suggested changes Sep 23, 2020
View reviewed changes
Copy link
Contributor

@hajya hajya left a comment

Choose a reason for hiding this comment

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

hmm. the logic looks fine, i'd love to avoid intertwining with internal or os src binaries/services where we can. Curious what you think.

build/internal/FactoryOrchestratorServiceTemplate.wm.xml Outdated Show resolved Hide resolved
src/CoreLibrary/IPCInterface.cs Outdated Show resolved Hide resolved
src/Service/ServiceExe.cs Outdated Show resolved Hide resolved
src/Service/ServiceExe.cs Show resolved Hide resolved
@spaceisfun spaceisfun requested a review from hajya September 23, 2020 18:50
hajya
hajya reviewed Sep 23, 2020
View reviewed changes
Copy link
Contributor

@hajya hajya left a comment

Choose a reason for hiding this comment

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

Consider going back and looking the rest of your change/code for the feedback Nuri gave. otherwise looks great!

@spaceisfun spaceisfun merged commit bafd93f into microsoft:main Sep 23, 2020
@spaceisfun spaceisfun deleted the user/jafriedm/ctrv2 branch September 23, 2020 20:00
spaceisfun pushed a commit to spaceisfun/FactoryOrchestrator that referenced this pull request Sep 26, 2020
Merged PR 5210037: merge public github to internal
9c4a2d8 
PRs 16,17,20

microsoft#16
microsoft#17
microsoft#20
hajya pushed a commit that referenced this pull request Oct 15, 2020
@spaceisfun @hajya
Get container info from FactoryOS registry instead of cmdiag; Create …
18f5c86 
…service events for container status (#20)

* Get container info from FactoryOS registry instead of cmdiag; Create service events for container status
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hajya hajya hajya approved these changes

Assignees

@spaceisfun spaceisfun

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Container logic should not depend on parsing console output
2 participants
@spaceisfun @hajya