-
-
Notifications
You must be signed in to change notification settings - Fork 197
Autocompletion #61
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
Autocompletion #61
Conversation
0dc855d
to
a27eefb
Compare
} | ||
|
||
if(scriptsOk) { | ||
console.log("Restart your shell to enable command auto-completion."); |
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.
Not a merge stopper, but why not using $logger?
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.
Due to copy paste
driven development 😄
334322c
to
5eb9f2f
Compare
|
||
private updateBashProfile(): IFuture<void> { | ||
return (() => { | ||
var callProfileScript = |
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.
Consider making this a constant (private static class member)
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.
Why?
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.
Do you change it? If so, it is clearly a var. But if it is a constant, lets make it official!
We discussed the issues in person and here's mine 👍. squash & go! |
5eb9f2f
to
91fb9ce
Compare
This PR should be merged after this telerik/mobile-cli-lib#33