Add target to approve new public API #1684
Conversation
|
Technically I think this target should be called "approve" and the current approve something like "check-api", but what can you do? |
I don't mind changing the name of the existing target to use |
There was a problem hiding this comment.
Nice! Thanks @blairconrad, I love the idea. I'm just not sure what the intended usage is (see comment)
tools/FakeItEasy.Build/Program.cs
Outdated
| @@ -1,5 +1,6 @@ | |||
| namespace FakeItEasy.Build | |||
| { | |||
| using System; | |||
There was a problem hiding this comment.
What do you need this for? As far as I can tell, the new code only uses classes from System.IO
There was a problem hiding this comment.
In initial development, I was just spewing messages to the console. I forgot to remove the using when I changed that.
tools/FakeItEasy.Build/Program.cs
Outdated
| "accept-api", | ||
| () => | ||
| { | ||
| foreach (var received in Directory.EnumerateFiles("tests/FakeItEasy.Tests.Approval", "*.received.txt", SearchOption.AllDirectories)) |
There was a problem hiding this comment.
Maybe specify /ApprovedApi in the path, to further reduce the search scope ?
validate-api feels slightly better… or we could leave as is. Or drop the whole thing if you like another approach, such as the environment variables. |
Or keep the name of the current target, and name the new one |
blairconrad
force-pushed
the
approve-new-api-task
branch
from
51e1aa2
to
d52bdbc
Compare
|
I think I made your suggested changes. And I squashed. If you like it enough, we could merge. If you have any doubts, we could not!! |
|
Thanks @blairconrad! |
|
Thank you! |
|
This change has been released as part of FakeItEasy 6.0.0-beta.1. |
… I get tired of copying files manually.