Actions
Bug #12884
closed[Composer] Failure to connect to git not well reported
Start date:
02/07/2018
Due date:
% Done:
100%
Estimated time:
(Total: 0.00 h)
Story points:
-
Release:
Release relationship:
Auto
Updated by Peter Amstutz almost 7 years ago
- Status changed from New to In Progress
Updated by Peter Amstutz almost 7 years ago
- Status changed from In Progress to New
Updated by Peter Amstutz almost 7 years ago
- Target version changed from 2018-01-17 Sprint to 2018-01-31 Sprint
Updated by Peter Amstutz almost 7 years ago
- Status changed from New to In Progress
- Target version changed from 2018-01-31 Sprint to 2018-02-14 Sprint
Updated by Peter Amstutz almost 7 years ago
Thought I fixed this but repository 404 error isn't being reported to the user, needs work.
Updated by Lucas Di Pentima almost 7 years ago
The updates LGTM. One question about code style:
- File src/app/core/panels/my-apps-panel/arvados-apps-panel.service.ts:
- Line 90: Could
load_dir()
function be declared only once outside the .map() of .map()s, for example right below thegetRootFolders()
level? At first glance it seems that it uses very few outside vars that could be passed as parameters, and we avoid declaring lots of times the same function and improve a lot the readability.
- Line 90: Could
Updated by Peter Amstutz almost 7 years ago
Lucas Di Pentima wrote:
The updates LGTM. One question about code style:
- File src/app/core/panels/my-apps-panel/arvados-apps-panel.service.ts:
- Line 90: Could
load_dir()
function be declared only once outside the .map() of .map()s, for example right below thegetRootFolders()
level? At first glance it seems that it uses very few outside vars that could be passed as parameters, and we avoid declaring lots of times the same function and improve a lot the readability.
It calls itself recursively as well as capturing some variables from the outer scope. You're right there's a readability tradeoff here.
Updated by Lucas Di Pentima almost 7 years ago
Just in case you're waiting for further feedback from me, this LGTM.
Updated by Peter Amstutz almost 7 years ago
- Status changed from In Progress to Resolved
Actions