Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include packages for x86/x64 onigwrap.dll, and dynamic binding betwee… #55

Merged
merged 2 commits into from
Jul 4, 2023

Conversation

Andrew1Medvedev
Copy link
Contributor

…n them #51

@danipen
Copy link
Owner

danipen commented Jun 9, 2023

Hi thanks for the contribution. Please take a look to the PR review comments.

@danipen
Copy link
Owner

danipen commented Jul 1, 2023

@Andrew1Medvedev did you manage to take a look at the comments? Thanks 🙏

@Andrew1Medvedev
Copy link
Contributor Author

I did not find any PR review comments. Could you please send me on my e-mail:
andrew.medvedev@alternetsoft.com

@@ -0,0 +1,3 @@
<Project>
<Import Project="$(MSBuildThisFileDirectory)\CopyNativeLibraries.targets" />
Copy link
Owner

Choose a reason for hiding this comment

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

I think is clearer to remove this file and reference directly CopyNativeLibraries.target from the TexmateSharp.csproj project (so it's explicit what we do).

Include="..\..\build\CopyNativeLibraries.targets" />
<Content Pack="True"
PackagePath="build"
Include="..\..\build\TextMateSharp.targets" />
Copy link
Owner

Choose a reason for hiding this comment

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

Why are we referencing both CopyNativeLibraries.tsrgets and TextmateSharp.targets here?

@Andrew1Medvedev
Copy link
Contributor Author

It is enough to reference only one *.targets, but it filename must be the same as the NuGet Package Id.
So I leave only TextMateSharp.targets

@danipen danipen merged commit 84a4383 into danipen:master Jul 4, 2023
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.

2 participants