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

Why does this depend on SourceLink? #3211

Closed
ericrrichards opened this issue Jan 2, 2020 · 6 comments
Closed

Why does this depend on SourceLink? #3211

ericrrichards opened this issue Jan 2, 2020 · 6 comments
Assignees
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository.

Comments

@ericrrichards
Copy link

ericrrichards commented Jan 2, 2020

Microsoft.Bot.Connector and Microsoft.Bot.Schema both depend on SourceLink.Create.CommandLine. Installing that NuGet package junks up one's project file by adding unnecessary build targets to run SourceLink, which can break builds if your project is hosted on VSTS rather than GitHub.

Unless there's a good reason that these Bot Framework packages should pull in that dependency, it's pretty unwelcome. I'm spending half a day going through project files and stripping out all of the gunk that the SourceLink NuGet package added.

@EricDahlvang
Copy link
Member

Related: #2992

BotBuilder-dotnet should be using https://github.com/dotnet/sourcelink

@vishwacsena vishwacsena added Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-reported Issue is created by anyone that is not a collaborator in the repository. labels Jan 2, 2020
@vishwacsena
Copy link
Contributor

@BruceHaley can you take a look and comment?

@gabog
Copy link
Contributor

gabog commented Jan 4, 2020

If we don't upgrade to Microsoft.SourceLink, I think we should at least mark the dependency as <PrivateAssets>all</PrivateAssets> so it doesn't flow to derived projects.

https://docs.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#controlling-dependency-assets

@srinaath srinaath added the customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. label Jan 8, 2020
@BruceHaley
Copy link
Contributor

BruceHaley commented Jan 15, 2020

It looks like upgrading to Microsoft.SourceLink #2992 needs to be done but will take some time.

If we want a more immediate solution perhaps we should also implement Gabo's suggestion now, that is mark the dependency <PrivateAssets>all</PrivateAssets>.

@BruceHaley
Copy link
Contributor

PR #3259 implements the immediate solution mentioned above.

@cleemullins
Copy link
Contributor

Fixed by #3259. Will release as part of R8 in March.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository.
Projects
None yet
Development

No branches or pull requests

8 participants