Build tweaks #1418
Build tweaks #1418
Conversation
build.cmd
Outdated
| @@ -1 +1,2 @@ | |||
| @dotnet run --project "%~dp0\tools\FakeItEasy.Build\FakeItEasy.Build.csproj" -- %* | |||
| @cd %~dp0 | |||
There was a problem hiding this comment.
Relative paths are used inside Program.cs. If you don't change working directory, then the targets will fail when run from a subdirectory, e.g. ../build.cmd.
There was a problem hiding this comment.
BTW since you currently need to be in the root folder to run the script, the current inclusion of %~dp0 in the path is redundant.
There was a problem hiding this comment.
Changing directories could be disorienting to the user. If we're going to support a "run the build from anywhere" kind of attitude, I think it's polite to pushd and then popd when we're done.
There was a problem hiding this comment.
@blairconrad it only changes it for the scope of the script:
There was a problem hiding this comment.
@adamralph that's only because you're running it from PowerShell. If you were running from cmd, it would stay in the changed directory. So I think pushd/popd is necessary.
There was a problem hiding this comment.
Ahhh, you're right! Thanks, I'll change it.
I guess I need to change this in my other projects too. Or I might just swap build.cmd for build.ps1 to force people to use PowerShell. 😉
There was a problem hiding this comment.
Or I might just swap build.cmd for build.ps1 to force people to use PowerShell. 😉
I'd consider doing that for FakeItEasy too... I never use cmd anymore.
| @@ -104,7 +104,7 @@ public static void Main(string[] args) | |||
| DependsOn("build", "outputDirectory", "pdbgit"), | |||
| () => | |||
| { | |||
| var version = Read(ToolPaths.GitVersion, "/showvariable NuGetVersionV2", "."); | |||
| var version = Read(ToolPaths.GitVersion, "/showvariable NuGetVersionV2"); | |||
There was a problem hiding this comment.
When SimpleExec is passed a working directory, it prints an extra line like so:
Working directory: .
Both this argument and this line are redundant, since "." refers to the current directory.
The other changes to remove ./ are pure refactoring (no observable effect).
|
BTW, you could choose to prefix all paths with |
There was a problem hiding this comment.
Thanks, @adamralph. It is tidier. And it's nice to have the latest stable Bullseye and SimpleExec.
I did ask for one change, but perhaps you and @thomaslevesque think it's foolish.
build.cmd
Outdated
| @@ -1 +1,2 @@ | |||
| @dotnet run --project "%~dp0\tools\FakeItEasy.Build\FakeItEasy.Build.csproj" -- %* | |||
| @cd %~dp0 | |||
There was a problem hiding this comment.
Changing directories could be disorienting to the user. If we're going to support a "run the build from anywhere" kind of attitude, I think it's polite to pushd and then popd when we're done.
build.cmd
Outdated
| @@ -1 +1,2 @@ | |||
| @dotnet run --project "%~dp0\tools\FakeItEasy.Build\FakeItEasy.Build.csproj" -- %* | |||
| @cd %~dp0 | |||
There was a problem hiding this comment.
@adamralph that's only because you're running it from PowerShell. If you were running from cmd, it would stay in the changed directory. So I think pushd/popd is necessary.
|
@thomaslevesque I swapped the |
👍 |
|
@blairconrad since your concerns were addressed, I'm taking the liberty of dismissing your review |
thomaslevesque
dismissed
blairconrad’s stale review
Requested changes have been made
|
Thanks @adamralph! |
|
Yes thanks, @adamralph. And @thomaslevesque, dismissing my review was the right move! |
|
This change has been released as part of FakeItEasy 4.8.1. Thanks, @adamralph. Look for your name in the release notes. 🏆 |
No description provided.