Skip to content

Remove unused files#6130

Merged
dipeshmsft merged 2 commits intodotnet:mainfrom
kant2002:kant/cleanup
May 9, 2022
Merged

Remove unused files#6130
dipeshmsft merged 2 commits intodotnet:mainfrom
kant2002:kant/cleanup

Conversation

@kant2002
Copy link
Copy Markdown
Contributor

Just general cleanup.
This dir.props is leftover from previous build infra.
makefiles also most likely do not used.

@kant2002 kant2002 requested a review from a team as a code owner February 15, 2022 18:20
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Feb 15, 2022
@ghost ghost requested review from SamBent, dipeshmsft and singhashish-wpf February 15, 2022 18:20
Copy link
Copy Markdown
Contributor

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

:shipit:

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 one looks odd...

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.

I do not have whole visibility for the project so was not sure about this file. Also changing define may touch a lot of files.

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 was a general observation, more aimed at the WPF team.

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.

I took a quick look and NETCOREAPP5_0 does not seem to be used and constants for the current target framework are declared by default (Example: NET6_0 for net6 target framework). I think it's safe to remove this one as well.

@kant2002
Copy link
Copy Markdown
Contributor Author

CLA check seems to be failing. I do not know why.

@RussKie
Copy link
Copy Markdown
Contributor

RussKie commented Feb 16, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@RussKie RussKie closed this Feb 16, 2022
@RussKie RussKie reopened this Feb 16, 2022
@ThomasGoulet73
Copy link
Copy Markdown
Contributor

ThomasGoulet73 commented Feb 16, 2022

The cla problem does not seem to be related to this PR, I have the same problem in my latest PR and there's bunch of PR with the same problem in dotnet/runtime.

@kant2002
Copy link
Copy Markdown
Contributor Author

Somebody else should do a review?

@vishalmsft
Copy link
Copy Markdown
Contributor

vishalmsft commented Feb 21, 2022

Seems native files are moved out from native folder to lib. This causes installer issues downstream.

From: runtimes\win-x86\native
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\runtimes\win-x86\native\D3DCompiler_47_cor3.dll
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\runtimes\win-x86\native\FXJITCompiler.pdb
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\runtimes\win-x86\native\PenImc_cor3.dll
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\runtimes\win-x86\native\PenImc_cor3.pdb
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\runtimes\win-x86\native\PresentationNative_cor3.dll
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\runtimes\win-x86\native\PresentationNative_cor3.pdb
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\runtimes\win-x86\native\vcruntime140d.dll
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\runtimes\win-x86\native\wpfgfx_cor3.dll
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\runtimes\win-x86\native\wpfgfx_cor3.pdb

To: lib\net7.0
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\lib\net7.0\D3DCompiler_47_cor3.dll
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\lib\net7.0\FXJITCompiler.pdb
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\lib\net7.0\PenImc_cor3.dll
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\lib\net7.0\PenImc_cor3.pdb
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\lib\net7.0\PresentationNative_cor3.dll
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\lib\net7.0\PresentationNative_cor3.pdb
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\lib\net7.0\vcruntime140d.dll
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\lib\net7.0\wpfgfx_cor3.dll
wpf\artifacts\packaging\Debug\Microsoft.DotNet.Wpf.GitHub.Debug\lib\net7.0\wpfgfx_cor3.pdb

cc: @singhashish-wpf

@RussKie
Copy link
Copy Markdown
Contributor

RussKie commented Feb 21, 2022

@kant2002 I suggest you build before and after the change and compare binlogs (under .\artifacts\logs), and see what's causing this change.

@kant2002
Copy link
Copy Markdown
Contributor Author

From what I see, this is comes down to old main from which I create this PR.

#5974 this line has the fix (at least how I understand this)
I did rebase, and issue seems to be gone.

@kant2002
Copy link
Copy Markdown
Contributor Author

kant2002 commented Mar 9, 2022

@vishalmsft can you take a look again on this change?

@dipeshmsft dipeshmsft self-assigned this Mar 22, 2022
@kant2002
Copy link
Copy Markdown
Contributor Author

@dipeshmsft what else needed for this PR to be merged? Itching to make future small improvements to WPF.

@dipeshmsft
Copy link
Copy Markdown
Member

Hey @kant2002 , we have started the test pass, as soon as it is finished, we will merge the PR.

@ghost ghost assigned kant2002 May 5, 2022
@dipeshmsft dipeshmsft merged commit 2c8611d into dotnet:main May 9, 2022
@kant2002 kant2002 deleted the kant/cleanup branch May 9, 2022 06:40
@ghost ghost locked as resolved and limited conversation to collaborators Jun 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants