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

Update MicrosoftPrivateIntellisensePackage and XmlDocFileRoot #38109

Merged
merged 3 commits into from
Jun 19, 2020

Conversation

carlossanlop
Copy link
Member

XmlDocFileRoot was defined in two places: packaging.props and docs.targets.
Moved it to a higher level to ensure it's only defined once, in Directory.Build.props. Made sure to define it after NuGetPackageRoot is defined.
Also made sure the folder it reads is net instead of netcoreapp, because the Docs build saves the .NET 5.0 intellisense files in the net folder, and the .NET 3.1 intellisense files in the netcoreapp folder.
Updated the nupkg to consume to the latest available version in the feed 20200602.3, which contains the Preview5 APIs.

I will make a similar change in the Preview6 branch today.
Thanks @safern for your help in figuring out what changes needed to be made.

@ghost
Copy link

ghost commented Jun 18, 2020

Tagging subscribers to this area: @ViktorHofer
Notify danmosemsft if you want to be subscribed.

@carlossanlop
Copy link
Member Author

Found a build error locally. Santi and I are investigating.

@safern
Copy link
Member

safern commented Jun 18, 2020

It seems like the only shared location were we could define XmlDocFileRoot is in Versions.props -- because this is used in pkgproj and Build.proj restore.

Co-authored-by: Santiago Fernandez Madero <[email protected]>
@carlossanlop carlossanlop requested a review from safern June 18, 2020 22:51
eng/Versions.props Outdated Show resolved Hide resolved
@ViktorHofer
Copy link
Member

For such shared properties we use Configurations.props.

@safern
Copy link
Member

safern commented Jun 19, 2020

We can’t use Configurations.props. We tried it already.

The reason for that is that when we’re building a pkgproj; arcade is imported after Configurations.props is imported and NugetPackageRoot is not defined at the moment this property is defined.

We disable importing arcade from the root for anything under src/libraries. The only place I could think of was Versions.props where in both code paths (Build.proj restore and pkgproj build) arcade would already had defined NugetPackageRoot

@ViktorHofer
Copy link
Member

Makes sense. We should enable the root Arcade import. The reason I had to disable that was because we relied on the old configuration system. I believe there aren't any blockers anymore.

@safern
Copy link
Member

safern commented Jun 19, 2020

Cool. Can we do that as part of another change? I believe there was an issue for that, right? I just don’t want to complicate this change.

@ViktorHofer
Copy link
Member

Sure

@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
@carlossanlop carlossanlop deleted the intellisense branch February 7, 2024 06:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants