Skip to content

MsBuildWorkspace small fixes#52390

Closed
jmarolf wants to merge 1 commit intodotnet:mainfrom
jmarolf:infra/msbuildworkspace-small-fixes
Closed

MsBuildWorkspace small fixes#52390
jmarolf wants to merge 1 commit intodotnet:mainfrom
jmarolf:infra/msbuildworkspace-small-fixes

Conversation

@jmarolf
Copy link
Contributor

@jmarolf jmarolf commented Apr 3, 2021

No description provided.

@jmarolf jmarolf requested a review from a team as a code owner April 3, 2021 03:55
@ghost ghost added the Area-IDE label Apr 3, 2021
Copy link
Contributor

Choose a reason for hiding this comment

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

lol.

Copy link
Contributor

Choose a reason for hiding this comment

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

the name before was correct. it is exposed outside the type, so should be pascal cased. _ is for privates.

Copy link
Contributor

Choose a reason for hiding this comment

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

this scares me. skipping for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

i triply hate ths now. having another type access an _ member of another varaible looks like it's somehow accessing a privat.e

Copy link
Contributor

Choose a reason for hiding this comment

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

honestly i dont' get why this is a switch, and not just return X || Y || Z;

@CyrusNajmabadi
Copy link
Contributor

So i ended up skpping this. lots of mechanical changes i approve of, and would accept a PR on. I don't like intermixing it with things taht are sublte and could change semantics. c an you break apart?

@sharwell
Copy link
Contributor

sharwell commented Apr 5, 2021

Marked as draft since it builds on #52389

@sharwell sharwell marked this pull request as draft April 5, 2021 13:21
@jmarolf jmarolf force-pushed the infra/msbuildworkspace-small-fixes branch from 4c6affb to db1d1c8 Compare April 8, 2021 02:21
@jmarolf jmarolf force-pushed the infra/msbuildworkspace-small-fixes branch from db1d1c8 to b50dd24 Compare April 8, 2021 19:45
@jmarolf jmarolf marked this pull request as ready for review April 8, 2021 20:12
@jmarolf
Copy link
Contributor Author

jmarolf commented Apr 8, 2021

splitting into different PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants