Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Project msbuild syntax cleanup #35686

Merged
merged 3 commits into from
Mar 3, 2019
Merged

Project msbuild syntax cleanup #35686

merged 3 commits into from
Mar 3, 2019

Conversation

ViktorHofer
Copy link
Member

@ViktorHofer ViktorHofer commented Mar 1, 2019 •
edited
Loading

Those attributes aren't needed anymore with the portable msbuild + our min version of msbuild desktop 15. Also removed the license headers that I added but which aren't needed as the marked files aren't shipping artifacts.

cc @ericstj @safern

@ViktorHofer ViktorHofer self-assigned this Mar 1, 2019
@ViktorHofer ViktorHofer force-pushed the ProjectCleanup branch 4 times, most recently from 28ba9bf to f01d954 Compare March 1, 2019 13:15
stephentoub
stephentoub approved these changes Mar 1, 2019
View reviewed changes
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

I looked at a smattering of the ~1000 files. I assume you did this programmatically and that all of the files got similar treatment? If so, LGTM.

@danmoseley
Copy link
Member

Good to see you're using the new function dotnet/msbuild#1277

@ViktorHofer
Copy link
Member Author

Unfortunately that function has a bug which makes it not very useful: dotnet/msbuild#2456 (comment)

@ViktorHofer
Copy link
Member Author

Yes I did regex replaces and checked most of it manually

ericstj
ericstj approved these changes Mar 1, 2019
View reviewed changes
Copy link
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

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

I spot checked. I'd recommend cleaning up DefaultTargets in the same change to avoid churning more than once. Thanks for doing this!

eng/Packaging.props Outdated
@@ -1,4 +1,4 @@
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Project DefaultTargets="Build">
Copy link
Member

Choose a reason for hiding this comment

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

You can remove DefaultTargets="Build", especially from props/targets files.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed that attribute from all props and targets file.

safern
safern reviewed Mar 1, 2019
View reviewed changes
Directory.Build.targets Outdated
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Member

Choose a reason for hiding this comment

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

I think we can also delete the entries, right @ericstj ?

Copy link
Member

Choose a reason for hiding this comment

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

Good point. MSBuild doesn't require this anymore. I guess it depends on wether or not all our tools which try to edit these are happy without the prolog. Seems like we'd be ok removing it and see what breaks and add it back. The only things that might break are maestro-like tools I think, or any crusty dev tools that are accessing projects without using MSBuild.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good to know, I wasn't sure if we still need them. Let's see.

safern
safern approved these changes Mar 1, 2019
View reviewed changes
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

Thanks!

@ViktorHofer ViktorHofer merged commit c390ce7 into dotnet:master Mar 3, 2019
@ViktorHofer ViktorHofer deleted the ProjectCleanup branch March 3, 2019 17:55
safern
safern reviewed Mar 4, 2019
View reviewed changes
src/Common/tests/Resources/Strings.resx
@@ -1,5 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if for .resx files it was still needed. @ericstj do you know?

Copy link
Member

Choose a reason for hiding this comment

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

Not off hand. Test VS resx designer (for editing) and build (for reading in our code generator and compilation in resgen).

Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/coreclr that referenced this pull request Mar 6, 2019
@ViktorHofer @Anipik
Project msbuild syntax cleanup (dotnet/corefx#35686)
e432f23 
* Remove license header from non-shipping files

* Remove obsolete Project attributes

* Remove xml header and remove DefaultTargets

Signed-off-by: dotnet-bot <anirudhagnihotry098@gmail.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corert that referenced this pull request Mar 6, 2019
@ViktorHofer @Anipik
Project msbuild syntax cleanup (dotnet/corefx#35686)
cc8857d 
* Remove license header from non-shipping files

* Remove obsolete Project attributes

* Remove xml header and remove DefaultTargets

Signed-off-by: dotnet-bot <anirudhagnihotry098@gmail.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/mono that referenced this pull request Mar 6, 2019
@ViktorHofer @Anipik
Project msbuild syntax cleanup (dotnet/corefx#35686)
61c5094 
* Remove license header from non-shipping files

* Remove obsolete Project attributes

* Remove xml header and remove DefaultTargets

Signed-off-by: dotnet-bot <anirudhagnihotry098@gmail.com>
marek-safar pushed a commit to mono/mono that referenced this pull request Mar 6, 2019
@ViktorHofer @marek-safar
Project msbuild syntax cleanup (dotnet/corefx#35686)
af3965d 
* Remove license header from non-shipping files

* Remove obsolete Project attributes

* Remove xml header and remove DefaultTargets

Signed-off-by: dotnet-bot <anirudhagnihotry098@gmail.com>
jkotas pushed a commit to dotnet/coreclr that referenced this pull request Mar 7, 2019
@ViktorHofer @jkotas
Project msbuild syntax cleanup (dotnet/corefx#35686)
864a9cc 
* Remove license header from non-shipping files

* Remove obsolete Project attributes

* Remove xml header and remove DefaultTargets

Signed-off-by: dotnet-bot <anirudhagnihotry098@gmail.com>
jkotas pushed a commit to dotnet/corert that referenced this pull request Mar 7, 2019
@ViktorHofer @jkotas
Project msbuild syntax cleanup (dotnet/corefx#35686)
0c94a0d 
* Remove license header from non-shipping files

* Remove obsolete Project attributes

* Remove xml header and remove DefaultTargets

Signed-off-by: dotnet-bot <anirudhagnihotry098@gmail.com>
@karelz karelz added this to the 3.0 milestone Mar 18, 2019
macrogreg pushed a commit to open-telemetry/opentelemetry-dotnet-instrumentation that referenced this pull request Sep 24, 2020
@ViktorHofer
Project msbuild syntax cleanup (dotnet/corefx#35686)
4d1bbb0 
* Remove license header from non-shipping files

* Remove obsolete Project attributes

* Remove xml header and remove DefaultTargets


Commit migrated from dotnet/corefx@c390ce7
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@ViktorHofer
Project msbuild syntax cleanup (dotnet/corefx#35686)
f8a8530 
* Remove license header from non-shipping files

* Remove obsolete Project attributes

* Remove xml header and remove DefaultTargets


Commit migrated from dotnet/corefx@c390ce7
This was referenced Apr 19, 2023
Closed
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ericstj ericstj ericstj approved these changes

@safern safern safern approved these changes

@stephentoub stephentoub stephentoub approved these changes

Assignees

@ViktorHofer ViktorHofer

Labels
area-Infrastructure-libraries
Projects
None yet
Milestone
3.0
Development

Successfully merging this pull request may close these issues.
6 participants
@ViktorHofer @danmoseley @stephentoub @ericstj @safern @karelz