Display prompt to restore all projects #2323

akshita31 · 2018-05-17T23:34:01Z

The prompt that asks to restore the project dependencies appears just once and clicking on "Restore" , restores all the projects.
The command palette has 2 options:

.NET : Restore Project - Displays a menu to select one project to be restored.
.NET : Restore Packages - Restores all the projects.

akshita31 · 2018-05-17T23:34:54Z

I am not sure what should be the names of the commands in the command pallette.

codecov · 2018-05-17T23:41:04Z

Codecov Report

Merging #2323 into master will increase coverage by 0.04%.
The diff coverage is 51.61%.

@@            Coverage Diff             @@
##           master    #2323      +/-   ##
==========================================
+ Coverage   62.21%   62.25%   +0.04%     
==========================================
  Files          86       86              
  Lines        3864     3855       -9     
  Branches      556      551       -5     
==========================================
- Hits         2404     2400       -4     
+ Misses       1299     1293       -6     
- Partials      161      162       +1

Flag	Coverage Δ
#integration	`51.34% <48.38%> (+0.06%)`	⬆️
#unit	`84.25% <100%> (-0.03%)`	⬇️

Impacted Files	Coverage Δ
src/observers/InformationMessageObserver.ts	`100% <100%> (ø)`	⬆️
src/observers/utils/ShowInformationMessage.ts	`83.33% <100%> (-4.17%)`	⬇️
src/features/commands.ts	`31.81% <46.42%> (+2.34%)`	⬆️
src/omnisharp/delayTracker.ts	`68.42% <0%> (-5.27%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f388323...e5a920f. Read the comment docs.

DustinCampbell · 2018-05-18T16:39:13Z

package.json

+        "category": ".NET"
+      },
+      {
+        "command": "dotnet.restore.all",
        "title": "Restore Packages",


Should we rename this to "Restore All Projects" for symmetry with "Restore Project"?

akshita31 · 2018-05-18T22:12:42Z

src/observers/InformationMessageObserver.ts

        let csharpConfig = this.vscode.workspace.getConfiguration('csharp');
        if (!csharpConfig.get<boolean>('suppressDotnetRestoreNotification')) {
-            let message = `There are unresolved dependencies from '${this.vscode.workspace.asRelativePath(event.unresolvedDependencies.FileName)}'. Please execute the restore command to continue.`;
-            return showInformationMessage(this.vscode, message, { title: 'Restore', command: 'dotnet.restore', args: event.unresolvedDependencies.FileName });
+            let message = `There are unresolved dependencies'. Please execute the restore command to continue.`;


@DustinCampbell Do we want to display the solution name ( the one we display in the status bar item) here -- There are unresolved dependencies from abc.sln ?

Maybe. If we do, the text should probably be in abc.sln rather than from abc.sln.

However, I'm interested in the semantics of how this message appears. If several "unresolved dependencies" events trigger for different projects, will we just display a single message? If so, would it be better if we triggered dotnet restore for each project rather than running it over the whole workspace? Doing the latter might be unexpected.

I think one problem is that discovery of unrestored projects is asynchronous. If we implement the feature so that it only restores projects for which it received the "unrestored project" message, there's a race condition between this popup and the discovery. Eg, one unrestored project is discovered and the warning is shown, and the user clicks restore. O# continues discovering projects and finds more, triggering a new prompt that the user needs to click to restore the newly discovered projects.

Restoring the entire sln file at once avoids that race condition. I suppose restoring the whole solution could be slow, but what else is "unexpected" about it?

There's also the case of multiple csproj's floating around with no sln file. I'm not sure how you would coordinate restoring all of those projects simulatenously without an sln...

rchande · 2018-05-24T21:08:47Z

src/features/commands.ts

-            dotnetRestoreAllProjects(server, eventStream);
-        }
-    }); 
+    let d4 = vscode.commands.registerCommand('dotnet.restore.project', () => pickProjectAndDotnetRestore(server, eventStream));


This is getting a bit silly. Can we just start pushing these into an array and construct our composite disposable from that?

rchande · 2018-05-24T21:17:14Z

src/features/commands.ts

+    let descriptors = await getProjectDescriptors(server);
+    eventStream.post(new CommandDotNetRestoreStart());
+    for (let descriptor of descriptors) {
+        await dotnetRestore(descriptor.Directory, eventStream);


What does the log output look like while we restore multiple projects?

It shows the logs from the restore of each project

rchande · 2018-06-08T16:12:52Z

@DustinCampbell @akshita31 Do we still have outstanding concerns about this PR or is it ready to merge? 😄

DustinCampbell · 2018-06-12T17:10:52Z

None from me.

akshita31 requested review from DustinCampbell, TheRealPiotrP, rchande, colombod and jonsequitur May 17, 2018 23:34

DustinCampbell approved these changes May 18, 2018

View reviewed changes

akshita31 commented May 18, 2018

View reviewed changes

akshita31 force-pushed the restore_solution branch from 81aa30b to dc55246 Compare May 24, 2018 18:27

rchande approved these changes May 24, 2018

View reviewed changes

akshita31 added 9 commits May 30, 2018 00:59

dotnet restore solution

b613ec5

remove using

9960947

Perfom restore on the solution

1d79866

use dotnet restore all

f624601

Renamed functions and added return types

931ca3d

remove args from info message

09e4231

Alter the unresolved dependencies message

5610f34

Rename to Restore all projects

ef58691

use a single disposable

ddb85ec

akshita31 force-pushed the restore_solution branch from 0e7c9dd to ddb85ec Compare May 29, 2018 19:33

Merge branch 'master' into restore_solution

73bd180

Merge branch 'master' into restore_solution

e5a920f

akshita31 merged commit 05fcd71 into dotnet:master Jun 18, 2018

akshita31 deleted the restore_solution branch June 18, 2018 17:43

akshita31 mentioned this pull request Jun 21, 2018

Add separate command for 'dotnet restore' on a specific project #240

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display prompt to restore all projects #2323

Display prompt to restore all projects #2323

akshita31 commented May 17, 2018 •

edited

Loading

akshita31 commented May 17, 2018

codecov bot commented May 17, 2018 •

edited

Loading

DustinCampbell May 18, 2018

akshita31 May 18, 2018

akshita31 May 18, 2018

DustinCampbell May 21, 2018

rchande May 29, 2018

rchande May 24, 2018

akshita31 May 29, 2018

rchande May 24, 2018

akshita31 May 29, 2018

rchande commented Jun 8, 2018

DustinCampbell commented Jun 12, 2018

Display prompt to restore all projects #2323

Display prompt to restore all projects #2323

Conversation

akshita31 commented May 17, 2018 • edited Loading

akshita31 commented May 17, 2018

codecov bot commented May 17, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rchande commented Jun 8, 2018

DustinCampbell commented Jun 12, 2018

akshita31 commented May 17, 2018 •

edited

Loading

codecov bot commented May 17, 2018 •

edited

Loading