Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add SourceLink support to package #42

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

Open
matracey wants to merge 12 commits into weichch:main
base: main
Choose a base branch
Loading
from matracey:dotnet-8-upgrade

Conversation

@matracey
Copy link
Contributor

@matracey matracey commented Nov 19, 2023
edited
Loading

New features and updates

522be95 add debug symbols and explicit packable configuration to src/Directory.build.props
f1fd5d7 add explicit SourceLink.GitHub package reference
31bd648 add msbuild properties for SourceLink
e72126c upgrade packages to latest versions

Project structure enhancements

558b8cb move sln file up to project root
f616e41 add missing loose items to sln file
f4ef2d5 add recommended VS Code extensions to extensions.json
8cecd49 update gitignore using dotnet new gitignore
7b1e292 add global.json with SDK version configuration
d554044 add IsTestProject property to Directory.Build.props for test folder
5ea3667 update AssemblyVersion and FileVersion to use JsonDiffPatchPackageVersion

@matracey matracey force-pushed the dotnet-8-upgrade branch 3 times, most recently from 30cfc99 to b2258ce Compare November 19, 2023 12:02
Copy link
Contributor Author

@weichch Are you still maintaining this package? Happy to take it over if not.

Copy link
Owner

weichch commented Dec 2, 2023

@weichch Are you still maintaining this package? Happy to take it over if not.

Hi @matracey yes I’m still maintaining this package. I’m on holiday for a few weeks will take a look once I’m back. Thanks for the PR btw.

Copy link
Owner

@weichch weichch left a comment

Choose a reason for hiding this comment

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

Hi @matracey Thanks again for taking effort to contribute. Like I mentioned in the comments, can we please track the repository house-keeping changes in a separate issue?

It's a little hard to follow the actual changes in this PR with more than 100 files changed due to reformatting.

@@ -0,0 +1,138 @@

Microsoft Visual Studio Solution File, Format Version 12.00
Copy link
Owner

@weichch weichch Jan 5, 2024

Choose a reason for hiding this comment

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

Why moved solution file here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it makes more sense to have the sln file at the root, since the src folder will contain the source for the library itself, and the test folder will contain the source code for the tests.

IMO, it's easier to find the sln file in the root, since many open source .NET repos follow this paradigm, and it makes more sense to reference like src\SystemTextJson.JsonDiffPatch\SystemTextJson.JsonDiffPatch.csproj & test\SystemTextJson.JsonDiffPatch.UnitTests\SystemTextJson.JsonDiffPatch.UnitTests.csproj rather than SystemTextJson.JsonDiffPatch\SystemTextJson.JsonDiffPatch.csproj & ..\test\SystemTextJson.JsonDiffPatch.UnitTests\SystemTextJson.JsonDiffPatch.UnitTests.csproj.

<PackageReference Include="MSTest.TestFramework" Version="2.1.2" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0">
Copy link
Owner

@weichch weichch Jan 5, 2024

Choose a reason for hiding this comment

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

What's the purpose of adding this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added SourceLink because it provides a great source debugging experience for end users. It provides a lot of debug-time assistance, allowing to step into the source code of the compiled NuGet package by pulling the source from GitHub, rather than trying to use decompilation. SourceLink is supported by Microsoft and included in the .NET SDK for a variety of common source control providers, including GitHub, and adding it provides a quick win for a much improved package consumer experience.

@weichch weichch mentioned this pull request Jan 20, 2024
Copy link
Owner

weichch commented Jan 20, 2024

I cherry picked some of the changes in this PR into #43 as co-authored commit. Let's keep this PR open for addressing VS Code support if you wish.

I also found Microsoft has added two new methods in .NET 8 JsonNode.DeepEquals and JsonNode.DeepClone. Those methods are conflicting with the extension methods in this package as per mentioned in #4, and I will take a look at how to address those. Meanwhile, those methods don't seem to exist in .NET 6 and .NET 7.

matracey reacted with thumbs up emoji

@weichch weichch changed the title (削除) Upgrade to .NET 8, dropping support for EOL frameworks (削除ここまで) (追記) Support VSCode and dotnet format (追記ここまで) Jan 21, 2024
@matracey matracey force-pushed the dotnet-8-upgrade branch 2 times, most recently from e03d069 to e72126c Compare January 22, 2024 22:24
@matracey matracey changed the title (削除) Support VSCode and dotnet format (削除ここまで) (追記) Add SourceLink support to package (追記ここまで) Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@weichch weichch weichch left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /