-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
CHE-3994 fix create workspace with project bug #4004
Conversation
Signed-off-by: Oleksii Orel <[email protected]>
qa tests for dashboard have passed |
@olexii4 @akurinnoy guys does this pr fixes also #3997 and #3995 ? |
Signed-off-by: Oleksii Kurinnyi <[email protected]>
@ashumilova @benoitf please review |
great, good job @akurinnoy! |
createWorkspace(workspaceConfig: any, attributes?: any): void { | ||
// TODO: no account in che ? it's null when testing on localhost | ||
createWorkspace(workspaceConfig: che.IWorkspaceConfig, attributes?: any): void { | ||
// tODO: no account in che ? it's null when testing on localhost |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK, it's to avoid warning from tslint
"comment-format": [
true,
"check-space",
"check-lowercase"
],
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant : you replaced TODO by tODO (first letter lowercase)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -1067,14 +973,14 @@ export class CreateProjectController { | |||
// generate a name | |||
|
|||
// starts with project | |||
var name = 'project'; | |||
let name = 'project'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let name : string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here name
is inferred to be string
because of its value.
Type Inference
Should we specify the type of the variable in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, up to you. In that case it's obvious but sometimes we declare variable used later.
* CHE-3994 fix create workspace with project bug Signed-off-by: Oleksii Orel <[email protected]> * fix RAM indicator Signed-off-by: Oleksii Kurinnyi <[email protected]>
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1859/ |
* CHE-3994 fix create workspace with project bug Signed-off-by: Oleksii Orel <[email protected]> * fix RAM indicator Signed-off-by: Oleksii Kurinnyi <[email protected]>
* CHE-3994 fix create workspace with project bug Signed-off-by: Oleksii Orel <[email protected]> * fix RAM indicator Signed-off-by: Oleksii Kurinnyi <[email protected]>
Signed-off-by: Oleksii Orel [email protected]
What does this PR do?
Fix create workspace with project bug.
What issues does this PR fix or reference?
#3994
#3995
#3996
Changelog