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

WIP: update package versions to use resolved version #1665

Closed
wants to merge 1 commit into from

Conversation

PatoBeltran
Copy link
Contributor

@PatoBeltran
Copy link
Contributor Author

@emgarten @jainaashish still a WIP, could you please give me feedback if this approach is the way to go or if there is a better way to achieve what I'm trying to do. Thanks!

@NuGet NuGet deleted a comment from dnfclas Aug 22, 2017
@@ -137,6 +142,7 @@ private PackageSourceMoniker SelectedSource

Loaded += (_, __) =>
{
ReadAssetsFileAndUpdateDependencyLookup();
Copy link
Contributor

Choose a reason for hiding this comment

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

reading assets file and updating dependency lookup should be part of search task which will make sure it doesn't delay the ui load.


_installedPackagesTask = PackageCollection.FromProjectsAsync(Projects, CancellationToken.None);
_installedPackagesTask = PackageCollection.FromProjectsAsync(Projects, null, CancellationToken.None);
_resolvedPackagesTask = PackageCollection.FromProjectsAsync(Projects, DependencyVersionLookup, CancellationToken.None);
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you really need two different package collections? I would expected it to be single resolved packages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first I only had one package collection, but the consolidate tab relies on the installed packages collection and when I used the resolved packages for this particular tab it didn't work. That's why I decided to do two package collections, any idea how this issue could be approached?


namespace NuGet.PackageManagement.VisualStudio
{
public class NuGetProjectDependencyVersionLookup
Copy link
Contributor

Choose a reason for hiding this comment

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

Why make it specific to build integrated projects? You can keep it generic to have all the resolved versions for each package across any kind of project. Just that in case of build integrated it will read from assets file vs for packages.config, it will be from config file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What the client does right now is to load the installed package identities from the config file (either project.json or packages.config, or csproj) and use them as a base for loading the UI. (Maybe I'm understanding something incorrectly?) What I'm trying to accomplish is updating those already loaded references with the resolved version. If this type is not specific to build integrated projects it would read the config file again (for packages.config projects) and try to update the values with the ones it already has.

At first I thought it would be a good idea to make this change from where the identities are loaded (for example BuildIntegratedPackageReference:54), but it seemed that going that direction was a more involved change on how package references are built for this type of projects.

@PatoBeltran PatoBeltran force-pushed the dev-patobeltran-package-reference-star-fix branch from 5d1fc22 to 9415900 Compare September 28, 2017 22:33
@emgarten
Copy link
Member

This looks like an old PR. This is a soft reminder to clean it up or close it until such time as it is active again.

@nkolev92 nkolev92 deleted the dev-patobeltran-package-reference-star-fix branch May 31, 2018 19:07
@nkolev92 nkolev92 restored the dev-patobeltran-package-reference-star-fix branch May 31, 2018 19:07
@nkolev92 nkolev92 deleted the dev-patobeltran-package-reference-star-fix branch May 31, 2018 19:07
@PatoBeltran PatoBeltran restored the dev-patobeltran-package-reference-star-fix branch July 3, 2018 22:11
@nkolev92 nkolev92 deleted the dev-patobeltran-package-reference-star-fix branch July 11, 2018 18:31
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.

4 participants