Skip to content

Removed now obsolete featureflag regarding envvars in ToolTask#7631

Merged
rokonec merged 1 commit into
dotnet:mainfrom
MeikTranel:dev/meiktranel/remove-dead-procstartinfoenv-featureflag
May 27, 2022
Merged

Removed now obsolete featureflag regarding envvars in ToolTask#7631
rokonec merged 1 commit into
dotnet:mainfrom
MeikTranel:dev/meiktranel/remove-dead-procstartinfoenv-featureflag

Conversation

@MeikTranel

Copy link
Copy Markdown
Contributor

Since ProcessStartInfo.Environment is now available in all supported TFMs,
the feature flag is now just obsolete and even more so an unnecessary
difference between core and framework versions of MSBuild.

Context

Found this old feature flag to be obsolete while researching ToolTask behavior.
This was added during the original OSS porting work of MSBuild, when ProcessStartInfo.Environment was only available
in .NET Core.

Changes Made

Use ProcessStartInfo.Environment for all flavors of MSBuild.

Testing

I don't think any additional testing code is needed - preexisting test suites cover this all pretty well and the change between Environment and EnvironmentVariable is only a change in type not an underlying change in behavior.

Notes

  1. EditorConfig enforced a couple of whitespace changes with trim_trailing_whitespace. I didn't remove these since that's supposed to happen at some point anyways. I hope that's ok.
  2. I didn't create an issue first since this is primarily a cosmetic change to reduce clutter and the change is relatively small - unified behavior between core and framework flavors only comes in second. Again, i hope that's cool with everyone 😅

Since `ProcessStartInfo.Environment` is now available in all supported TFMs,
the feature flag is now just obsolete and even more so an unnecessary
difference between core and framework versions of msbuild.

@Forgind Forgind 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.

Sounds good to me, thanks!

@rainersigwald rainersigwald added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label May 23, 2022
@rainersigwald rainersigwald added this to the VS 17.3 milestone May 23, 2022
@rokonec rokonec merged commit f1dae6a into dotnet:main May 27, 2022
@MeikTranel MeikTranel deleted the dev/meiktranel/remove-dead-procstartinfoenv-featureflag branch May 27, 2022 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants