Skip to content

Adding MSBuild support for transitive and multi-target scenarios#1834

Merged
belav merged 4 commits intobelav:mainfrom
chrisdrobison-fbg:enable-transitive-support
Mar 26, 2026
Merged

Adding MSBuild support for transitive and multi-target scenarios#1834
belav merged 4 commits intobelav:mainfrom
chrisdrobison-fbg:enable-transitive-support

Conversation

@chrisdrobison-fbg
Copy link
Copy Markdown
Contributor

Description

Adding buildTransitive and buildMultiTarget so CSharpier can still work as a transitive dependency.

Related Issue

Fixes: #1833

Checklist

  • My code follows the project's code style
    • always var
    • follow existing naming conventions
    • always this.
    • no pointless comments
  • I will not force push after a code review of my PR has started
  • I have added tests that cover my changes

Copy link
Copy Markdown
Owner

@belav belav left a comment

Choose a reason for hiding this comment

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

I fixed the unrelated warning on main and got that merged into here.

Could you fix the formatting or see if my comment about pulling in the existing folders works?

Thanks!

Comment on lines +24 to +25
<Content Include="buildMultiTarget/*" PackagePath="buildMultiTarget/" />
<Content Include="buildTransitive/*" PackagePath="buildTransitive/" />
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I was gonna approve and merge as is, but because this failed formatting checks I'm curious.

Could this instead just pull the existing files into those paths? It seems like this may work without needing the 4 new files. Either way is fine with me, I don't expect to be adding any new files to those folders.

    <Content Include="build/*" PackagePath="buildMultiTarget/" />
    <Content Include="build/*" PackagePath="buildTransitive/" />

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's perfectly okay. Just wasn't sure if you wanted a least a little separation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Copy Markdown
Owner

@belav belav left a comment

Choose a reason for hiding this comment

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

LGTM!

@belav belav merged commit bc1979c into belav:main Mar 26, 2026
6 of 7 checks passed
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.

[Feature]: Add MSBuild transitive and multi-target support

2 participants