Skip to content

cleanup some OOP code #25937

Merged
heejaechang merged 6 commits intodotnet:masterfrom
heejaechang:cleanupOOP
Apr 7, 2018
Merged

cleanup some OOP code #25937
heejaechang merged 6 commits intodotnet:masterfrom
heejaechang:cleanupOOP

Conversation

@heejaechang
Copy link
Copy Markdown
Contributor

Customer scenario

There is no customer experience change

Bugs this fixes

N/A

Workarounds, if any

N/A

Risk

N/A

Performance impact

N/A

Is this a regression from a previous update?

N/A

Root cause analysis

just moving around code. no functional or perf changes.

How was the bug found?

N/A

@heejaechang heejaechang requested a review from a team as a code owner April 4, 2018 20:13
@heejaechang
Copy link
Copy Markdown
Contributor Author

@jinujoseph @dotnet/roslyn-ide @dotnet/roslyn-analysis can i get code review? this doesn't change anything just moving code around.

@heejaechang
Copy link
Copy Markdown
Contributor Author

@dotnet/roslyn-infrastructure I am not sure what is failing.

@heejaechang
Copy link
Copy Markdown
Contributor Author

retest windows_debug_unit64_prtest please

@heejaechang
Copy link
Copy Markdown
Contributor Author

retest windows_release_unit64_prtest please

@heejaechang
Copy link
Copy Markdown
Contributor Author

test failures known issue - #25931

I am working on it.

@heejaechang
Copy link
Copy Markdown
Contributor Author

@Pilchie approval for 15.7?

@Pilchie
Copy link
Copy Markdown
Member

Pilchie commented Apr 5, 2018

I don't see the need to ship this in 15.7. Is there more work planned that depends on it? If not, I would suggest just putting this in master.

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.

❗️ This is mitigated in #25938; no need to disable here.

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.

did these methods change? or were these only moves?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

moved

@heejaechang heejaechang dismissed sharwell’s stale review April 5, 2018 09:44

removed skipped. sent out PR for the flaky test

@heejaechang
Copy link
Copy Markdown
Contributor Author

retest this please

@heejaechang
Copy link
Copy Markdown
Contributor Author

myget is not responding.

@heejaechang heejaechang changed the base branch from dev15.7.x to master April 6, 2018 19:16
@heejaechang
Copy link
Copy Markdown
Contributor Author

moving to master. not taking to 15.7

@heejaechang
Copy link
Copy Markdown
Contributor Author

retest windows_release_unit64_prtest please

@heejaechang
Copy link
Copy Markdown
Contributor Author

@jinujoseph this is master so 1 approval is all I need right?

@jinujoseph
Copy link
Copy Markdown
Contributor

Yes

@heejaechang heejaechang merged commit 9932fd5 into dotnet:master Apr 7, 2018
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.

5 participants