Skip to content

[WIP] Envvars #648

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
Closed
maneely wants to merge 11 commits into from
Closed

[WIP] Envvars #648

maneely wants to merge 11 commits into from

Conversation

@maneely
Copy link
Contributor

@maneely maneely commented Jul 23, 2019 •
edited
Loading

[WIP] Current thoughts on integrating changes from this pending PR in msbuild:
dotnet/msbuild#4461

I assume we'll discuss today. :)

@smera , @cdmihai , @AndyGerlicher

smera
smera reviewed Jul 23, 2019
View reviewed changes
smera
smera reviewed Jul 23, 2019
View reviewed changes
@smera
Copy link
Member

smera commented Jul 23, 2019

using System;

nit: missing headers and comments in general

Refers to: Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs:1 in 74a7572. [](commit_id = 74a7572, deletion_comment = False)

smera
smera reviewed Jul 23, 2019
View reviewed changes
smera
smera reviewed Jul 23, 2019
View reviewed changes
dannyvv
dannyvv reviewed Jul 24, 2019
View reviewed changes
dannyvv
dannyvv reviewed Jul 24, 2019
View reviewed changes
dannyvv
dannyvv reviewed Jul 24, 2019
View reviewed changes
dannyvv
dannyvv approved these changes Jul 24, 2019
View reviewed changes
Copy link
Member

@dannyvv dannyvv left a comment

Choose a reason for hiding this comment

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

:shipit:

@maneely
Copy link
Contributor Author

maneely commented Jul 29, 2019

Clarifying note on this PR:
Currently this is just to collaborate on how we want the changes to look/behave. There is more work to do (e.g. UTs) and there is work this is depending on that hasn't been merged into MSBuild yet.

@olkononenko olkononenko changed the title Envvars [WIP] Envvars Aug 8, 2019
@olkononenko
Copy link
Member

olkononenko commented Aug 8, 2019 •
edited
Loading

Moved 'WIP' into the title to prevent an accidental merge.

@narasamdya narasamdya closed this Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smera smera smera left review comments

+1 more reviewer

@dannyvv dannyvv dannyvv approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@maneely @smera @olkononenko @dannyvv @narasamdya