Skip to content

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

Merged
merged 1 commit into from
Aug 20, 2014
Merged

Autocompletion #61

merged 1 commit into from
Aug 20, 2014

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented Aug 20, 2014

This PR should be merged after this telerik/mobile-cli-lib#33

@Fatme Fatme force-pushed the fatme/autocompletion branch from 0dc855d to a27eefb Compare August 20, 2014 06:41
}

if(scriptsOk) {
console.log("Restart your shell to enable command auto-completion.");
Copy link
Contributor

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?

Copy link
Contributor Author

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 😄

@Fatme Fatme force-pushed the fatme/autocompletion branch from 334322c to 5eb9f2f Compare August 20, 2014 09:01

private updateBashProfile(): IFuture<void> {
return (() => {
var callProfileScript =
Copy link
Contributor

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor

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!

@teobugslayer
Copy link
Contributor

We discussed the issues in person and here's mine 👍. squash & go!

@Fatme Fatme force-pushed the fatme/autocompletion branch from 5eb9f2f to 91fb9ce Compare August 20, 2014 09:17
Fatme added a commit that referenced this pull request Aug 20, 2014
@Fatme Fatme merged commit 48a8393 into master Aug 20, 2014
@Fatme Fatme deleted the fatme/autocompletion branch August 20, 2014 09:22
rigor789 pushed a commit that referenced this pull request Feb 7, 2022
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.

2 participants