Skip to content
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.

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

Replaced GetDirectoryNameOfFileAbove with GetPathOfFileAbove #43462

Closed
wants to merge 7 commits into from

Conversation

TaurahSP
Copy link

Replaced "$([MSBuild]::GetDirectoryNameOfFileAbove(" with "$([MSBuild……]::GetPathOfFileAbove(" to improve readability

@dnfadmin
Copy link

dnfadmin commented Oct 15, 2020

CLA assistant check
All CLA requirements met.

@TaurahSP
Copy link
Author

@marek-safar can you help please, any idea why the build is failing

@marek-safar
Copy link
Contributor

You can ignore the failure, it's infrastructure issue

@danmoseley
Copy link
Member

This is for #30468

Hi @TaurahSP , thanks for the contribution, but there are a few issues here

  1. You have included some files that didn't need to change, eg json, txt
  2. The transformation proposed only works on this pattern:
    $([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), FOO))\FOO
    then it can be changed to
    $([MSBuild]::GetPathOfFileAbove(FOO))
    It doesn't work where there are different files in the same line, or one is a directory. So the edits to testprojects.props and project.csproj.template aren't valid, for those reasons respectively.

I am going to close this PR and feel free to create a new one if you want to give another shot. Or, you could choose to pick a different issue as you prefer.

@danmoseley danmoseley closed this Dec 15, 2020
@danmoseley
Copy link
Member

@ghost ghost locked as resolved and limited conversation to collaborators Jan 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants