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

Enable source link support. #884 #885

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davidroth
Copy link
Contributor

@davidroth davidroth commented Jan 13, 2021

These are the changes which are required for source link. (#884)
For information about the properties, see https://github.com/dotnet/sourcelink/blob/master/docs/README.md

I can rename the $(GITHUB_ACTIONS) variable, since you do not use Github_Actions or any other CI system.
How do you publish SimpleInjector? Locally from your PC?

When using dotnet pack, the following packages are created:

  • SimpleInjector.[version].nupkg
  • Simpleinjector.[version].snupkg

You can open SimpleInjector.[version].nupkg in Nuget Package explorer.
Notice the difference between the new package (left) and the old (right). Lots of green checks indicating that everything is ok.

image

More important, stepping into SimpleInjector source is now possible 🤘

image

Thats all :-)

@dotnetjunkie
Copy link
Collaborator

Thanks for this PR. I, indeed, publish the libraries from my local PC.

looking at your PR, however, I realize that more changes might be required to set this up, because currently NuGet packages are not built using dotnet pack. This is mainly for historical reasons, but also because of customizations in the NuGet package that are not supported by dotnet pack (probably main thing is lack of allowing to add a upper limit in package references, although this does not apply for the core library itself). Instead, currently this is done using a manually crafted template that gets copied, updated, appended, and zipped using the build script.

I'm not married with the way it is currently done. As a matter of fact, it is pretty ugly and if we can replace it with a simple dotnet pack command, the better. But this might take some more testing to make sure the packages are constructed identically.

I'll keep this PR open and get back to this after I were able to update packaging to dotnet pack.

Thanks again.

@davidroth
Copy link
Contributor Author

@dotnetjunkie Any updates on this? With the new "External Sources" supports of VS 2022, this gets even more interesting ;-)

@dotnetjunkie
Copy link
Collaborator

dotnetjunkie commented Sep 2, 2021

No progress on this. Don't expect this feature any time soon, considering my current time constraints.

If you want to speed things up, you could cook up a (second) pull request that allows the NuGet packages in the main repository to be auto-generated. Although, on itself, this isn't that hard, the catch here are the NuGet package dependencies. The SimpleInjector.Packaging v5.x package, for instance, should not be able to depend on SimpleInjector v6.x. Although this is something that can be configured in the package's nuspec file, the VS package generator does not support this feature. So you'll have to come up with a way to circumvent this, because this is an important configuration of a package.

With that in place, source link can be added on top.

@davidroth davidroth closed this Mar 29, 2022
@dotnetjunkie
Copy link
Collaborator

I reopened this issue because, although there is no progress in this regard, source link support is still something I'd like to add in the future.

@dotnetjunkie dotnetjunkie reopened this Mar 29, 2022
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