Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Add parameters' type information in JSDoc comments scaffolding#12722

Merged
anubmat merged 6 commits into
mastermicrosoft/TypeScript:masterfrom
correctCommentsScaffoldingmicrosoft/TypeScript:correctCommentsScaffoldingCopy head branch name to clipboard
Dec 8, 2016
Merged

Add parameters' type information in JSDoc comments scaffolding#12722
anubmat merged 6 commits into
mastermicrosoft/TypeScript:masterfrom
correctCommentsScaffoldingmicrosoft/TypeScript:correctCommentsScaffoldingCopy head branch name to clipboard

Conversation

@anubmat

@anubmat anubmat commented Dec 7, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

Adds type {any} in comments for parameters in JS and JSX files
image

Earlier type was missing:
image

@msftclas

msftclas commented Dec 7, 2016

Copy link
Copy Markdown

Hi @anubmat, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Anubha Mathur). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@anubmat

anubmat commented Dec 8, 2016

Copy link
Copy Markdown
Contributor Author

@jramsay , @billti , @vlad, Can someone please review?

DanielRosenwasser
DanielRosenwasser reviewed Dec 8, 2016
View reviewed changes
Comment thread src/services/jsDoc.ts Outdated
const lineStart = sourceFile.getLineStarts()[posLineAndChar.line];

const indentationStr = sourceFile.text.substr(lineStart, posLineAndChar.character);
const isJavaScriptFile = hasJavaScriptFileExtension(sourceFile.fileName); // JSX or JS file

@DanielRosenwasser DanielRosenwasser Dec 8, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think that this comment adds anything.

@anubmat anubmat Dec 8, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

agreed.. added because of jsx files... will remove.

@DanielRosenwasser

DanielRosenwasser commented Dec 8, 2016

Copy link
Copy Markdown
Member

👍 after addressing my one nit.

@billti

billti commented Dec 8, 2016

Copy link
Copy Markdown
Member

Looks good for now. We might want to make this behavior optional (based on a editor config setting) if we get feedback that some folks don't like it. I'd assume some devs don't want the type annotations in their JsDoc comments. Thanks! 👍

@anubmat anubmat merged commit 07ce4bd into master Dec 8, 2016
@anubmat anubmat deleted the correctCommentsScaffolding branch December 8, 2016 19:38
@mhegazy mhegazy mentioned this pull request Dec 12, 2016
Merged
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DanielRosenwasser DanielRosenwasser DanielRosenwasser left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@anubmat @msftclas @DanielRosenwasser @billti
Morty Proxy This is a proxified and sanitized view of the page, visit original site.