Conversation
SteveSandersonMS
left a comment
SteveSandersonMS
left a comment
There was a problem hiding this comment.
Thanks Nate! Mostly this looks great.
As per my inline comment, could we go back to having each of the src projects declare its own values for the following independently?
<ItemGroup>
<None Remove="node_modules\**\*" />
<EmbeddedResource Include="Content\**\*" />
</ItemGroup>
<Target Name="PrepublishScript" BeforeTargets="PrepareForPublish" Condition=" '$(IsCrossTargetingBuild)' != 'true' ">
<Exec Command="npm install" />
<Exec Command="node node_modules/webpack/bin/webpack.js" />
</Target>
Apart from that this looks good to me!
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <None Remove="node_modules\**\*" /> |
There was a problem hiding this comment.
I'm not keen on this or any of the following lines in this csproj being moved out into Directory.Build.props, because:
- It's hard to reason about which
node_modulesorContentfiles are being included/excluded when the paths aren't even relative to the file that's referencing them - It makes an assumption about conventions for inclusion/exclusion and prepublishing always being the same across every project in
src, which isn't guaranteed. Although they might all happen to follow the same conventions today, there's no reason to believe that should always be so. Not all of them should necessarily have aContentdirectory, usenode_modules, etc.
So I would prefer not to deduplicate these values, but rather keep the src projects equivalently decoupled as they were before.
There was a problem hiding this comment.
Sounds good. I'll revert this piece of the change.
natemcmaster
force-pushed
the
namc/dirbuild
branch
from
8f6ba58 to
c6a9a45
Compare
|
🆙 📅 |
|
I think I address your feedback, so I'm going to merge this so I can get #1236 in too. If there was anything I missed, let me know and I'll fix it. |
3 participants
We've started using Directory.Build.props/targets across aspnet repos to reduce the duplication between csproj files, and reduce the amount of boilerplate code required to add new projects and tests.