Skip to content
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

Feature/sqlcmd : Enable running scripts with SQLCMD variables - Part 1 #839

Merged
merged 7 commits into from
Aug 9, 2019

Conversation

udeeshagautam
Copy link
Contributor

This contains:

  1. Changes to enable running of a script file containing (and defined) SQLCMD variables. This will evolve to enable getting arguments from outside the script in later changes.
  2. Changes to switch on/off Intellisense with changing SQLCMD mode. We will make changes to better handle the specific SQLCMD Intellisense later.

@coveralls
Copy link

coveralls commented Jul 31, 2019

Coverage Status

Coverage increased (+0.3%) to 76.759% when pulling 516c54c on feature/sqlcmd into ec09ad2 on master.

Copy link
Member

@pensivebrian pensivebrian left a comment

Choose a reason for hiding this comment

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

:shipit:

@ranasaria
Copy link
Contributor

        return Task.FromResult(true);

even though this is not your change I suggest changing returns of Task.FromResult(true) to Task.CompletedTask. The previous code was the best option before .Net 4.6. But from .Net 4.6 it is best to use this predefined constant.

Here is some discussion on this topic:
https://stackoverflow.com/questions/13127177/if-my-interface-must-return-task-what-is-the-best-way-to-have-a-no-operation-imp


Refers to: src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs:1758 in 26eca53. [](commit_id = 26eca53, deletion_comment = False)

@udeeshagautam udeeshagautam merged commit 68145d5 into master Aug 9, 2019
@udeeshagautam udeeshagautam deleted the feature/sqlcmd branch August 9, 2019 21:26
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.

None yet

6 participants