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

fix: #55946 Added comment snippet variable #63572

Merged
merged 4 commits into from Nov 22, 2018

Conversation

@karanisverma
Copy link
Contributor

commented Nov 21, 2018

Added comment snippet variable
it fixes following issue
#55946

@msftclas

This comment has been minimized.

Copy link

commented Nov 21, 2018

CLA assistant check
All CLA requirements met.

} else if (name === 'BLOCK_COMMENT_START') {
return comments.blockCommentStartToken;
} else if (name === 'BLOCK_COMMENT_END') {
return comments.blockCommentEndToken;

This comment has been minimized.

Copy link
@jrieken

jrieken Nov 21, 2018

Member

... || undefined to ensure the default value is applied (in all cases)

This comment has been minimized.

Copy link
@karanisverma

karanisverma Nov 21, 2018

Author Contributor

Thanks, I have made changes as per your suggestion.

@jrieken jrieken self-assigned this Nov 21, 2018

@jrieken jrieken added this to the November 2018 milestone Nov 21, 2018

@jrieken

This comment has been minimized.

Copy link
Member

commented Nov 21, 2018

@karanisverma The build has failed due to a strict null error. You can them locally via F1 > Run Task > Strict Null Checks - it will then squiggle the code for null-violations

@karanisverma

This comment has been minimized.

Copy link
Contributor Author

commented Nov 21, 2018

Thanks for the info, Looking into it :)

@karanisverma

This comment has been minimized.

Copy link
Contributor Author

commented Nov 21, 2018

@jrieken I have fixed the issue and I have checked locally by following the steps you mentioned above.
Sharing log for test below:

> Executing task: npm run strict-null-check <


> code-oss-dev@1.30.0 strict-null-check /Users/karanverma/me/oss/vscode
> tsc -p src/tsconfig.strictNullChecks.json


Terminal will be reused by tasks, press any key to close it.

build process seems to fail in following file
src/vs/base/browser/ui/tree/abstractTree.ts(205,12): error TS2532: Object is possibly 'undefined'.

I guess it's not because of the changes in this PR. Please let me know if there is anything I have to fix at my end.

@karanisverma

This comment has been minimized.

Copy link
Contributor Author

commented Nov 22, 2018

@jrieken I have pulled code from master, tests are passing now 🙌🏼

@jrieken
Copy link
Member

left a comment

lgtm

@jrieken jrieken merged commit f325118 into microsoft:master Nov 22, 2018

2 checks passed

VS Code #20181121.50 succeeded
Details
license/cla All CLA requirements met.
Details
@jrieken

This comment has been minimized.

Copy link
Member

commented Nov 22, 2018

Thanks @karanisverma

@karanisverma

This comment has been minimized.

Copy link
Contributor Author

commented Nov 22, 2018

Anytime :), Thanks you for guiding me 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.