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

pathFilters paths should be relative to the version.json #465

Merged
merged 4 commits into from
Apr 18, 2020

Conversation

saul
Copy link
Contributor

@saul saul commented Apr 12, 2020

Fixes #451. Path were incorrectly relative to the project directory.

This PR ended up being a bit more an invasive change than I initially anticipated. This is because the JSON serialisation code understandably had no notion of where the version.json was being written to.

Changes

  • VersionFile.SetVersion/GetVersion must point to version.json files that reside in a Git repository to be able to serialize/deserialize path filters
  • FilterPath is now part of the public API, and is used on the VersionFile class
  • Added lots of tests :)

Fixes dotnet#451

Path were incorrectly relative to the project directory.
Copy link
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

Thanks so much for fixing this. I see what you mean by it being pretty invasive, but I support what you're doing. Just a few touch-ups and your own TODO comments to resolve and I'll merge.

src/NerdBank.GitVersioning/VersionOptions.cs Show resolved Hide resolved
src/NerdBank.GitVersioning/GitExtensions.cs Outdated Show resolved Hide resolved
src/NerdBank.GitVersioning/VersionFile.cs Outdated Show resolved Hide resolved
src/NerdBank.GitVersioning.Tests/VersionFileTests.cs Outdated Show resolved Hide resolved
src/NerdBank.GitVersioning.Tests/VersionFileTests.cs Outdated Show resolved Hide resolved
@saul saul requested a review from AArnott April 13, 2020 10:35
@AArnott
Copy link
Collaborator

AArnott commented Apr 13, 2020

Thanks, @saul. This is looking good. But you have evidently introduced a test failure in GetBuildVersion_OutsideGit_PointingToGit. Can you look into that?

@saul
Copy link
Contributor Author

saul commented Apr 13, 2020

Unfortunately I can't run those tests, even on master I get:

BuildIntegrationTests.GetBuildVersion_OutsideGit_PointingToGit

System.IO.FileLoadException: The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131040)

System.IO.FileLoadException
The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131040)
   at BuildIntegrationTests.Init()
   at BuildIntegrationTests..ctor(ITestOutputHelper logger) in C:\Code\Nerdbank.GitVersioning\src\NerdBank.GitVersioning.Tests\BuildIntegrationTests.cs:line 55

@AArnott
Copy link
Collaborator

AArnott commented Apr 13, 2020

OK, I'll look into it then. Thanks.

@AArnott
Copy link
Collaborator

AArnott commented Apr 18, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

That they fail might be understood, but why they're failing only *now* is the question.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nuget Package Version and nbgv cloud NBGV_NuGetPackageVersion differ
2 participants