Added warning when generating new files #28
Conversation
|
As it is now the build will continue and if successful the project will require a re-build (dotnet build will regard the project as up to date so won't rebuild it by itself) so there might be a case to make that we should attempt to abort (non-zero exitcode?) the build to make the rebuilding step simpler. |
|
I think I'd be in favour of a non-zero exitcode (assuming that marks the build as failed?) Also, I vote for the shorter file names in the warning messages you've proposed. See what @stajs thinks though. |
| featureFiles = directory.GetFiles("*.feature", SearchOption.AllDirectories); | ||
| var generatedFileExists = featureFiles.Where(f => !File.Exists(f.FullName + ".cs")).ToList(); | ||
|
|
||
| var xproj = GetXproj(directory); |
There was a problem hiding this comment.
Definitely need to reach an agreement with @stajs regarding spacing. 😄 Personally I use (and recommend) Keep Tabs in *.cs
There was a problem hiding this comment.
Happy to adapt these diffs look terrible :( I changed my vs tab settings but would like to keep settings for work projects and such. Perhaps https://visualstudiogallery.msdn.microsoft.com/c8bccfe2-650c-4b42-bc5c-845e21f96328 is an acceptable solution?
We'll see if tab settings will fix this issue next PR:
There was a problem hiding this comment.
I use the editorconfig extension so would certainly work for me. To be fair, for small projects like this I tend to use VSCode which automatically uses editorconfig files I believe (or perhaps I installed something and forgot...).
There was a problem hiding this comment.
I added an issue(#29) so it can be handled under a formatting only PR.
| @@ -61,7 +61,7 @@ public void Fix(DirectoryInfo directory) | |||
|
|
|||
| private void WarnNotExists(FileInfo featureFile) | |||
| { | |||
| WriteLine("New file generated: " + featureFile.FullName + ".cs. No tests in " + featureFile.Name + " will be discovered by dotnet test"); | |||
| WriteLine($@"New file generated: {featureFile.FullName}.cs. No tests in {featureFile.Name} will be discovered by dotnet test"); | |||
There was a problem hiding this comment.
It's only trivial, but you don't need the literal (@) marker here.
There was a problem hiding this comment.
Oops. Its a habit from my work project which uses style cop and style cop blows up on interpolation unless you use the marker. I'll fix it tomorrow, going to bed now.
|
Awesome guys! 🍻 Message length: generally I'm in favor of verbosity, especially when it comes to tracking down a file. Being able to copy a whole path and WIN-R, CTRL-V to find it is something I personally use a lot. And seeing as this is only a temporary warning that goes away on next build, I'm inclined to leave it. Let's just see how it feels after using for a while. Whitespace: this usually triggers a religious war. Editorconfig looks good, I'm happy to add that in. Non-zero exit code: it would be good if we can fail the build. Color: nice to have, but a bonus. |
Fixes #25
Warnings look like this:
New file generated: Y:\Projects\SpecFlow.Dnx\samples\VS2015\SpecFlow 2.1.0\net461\Sample.Website.Tests\Root.feature.cs. No tests in Root.feature will be discovered by dotnet testThings that may need improvement:
featureFile.FullName.Replace(directory.FullName + Path.DirectorySeparatorChar, ""). Would produce warning like this:New file generated: Folder\Root.feature.cs. No tests in Root.feature will be discovered by dotnet test