Skip to content

New rule: requireReturnDescription#125

Closed
weekens wants to merge 3 commits intojscs-dev:masterfrom
weekens:master
Closed

New rule: requireReturnDescription#125
weekens wants to merge 3 commits intojscs-dev:masterfrom
weekens:master

Conversation

@weekens
Copy link
Copy Markdown
Contributor

@weekens weekens commented Jul 12, 2015

No description provided.

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.

invalid? huh.

@qfox
Copy link
Copy Markdown
Member

qfox commented Jul 12, 2015

Looks good at all ;-) Not sure why you called 2 cases invalid jsdoc. Can you elaborate?

@weekens
Copy link
Copy Markdown
Contributor Author

weekens commented Jul 13, 2015

Well, it's totally analogous to require-param-description.js test. We check that JSCS does NOT report invalid JSDoc in valid case. How would you name the test?

@weekens
Copy link
Copy Markdown
Contributor Author

weekens commented Jul 26, 2015

@zxqfox, any news on this request?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as above typo! :)

@qfox qfox closed this in e381c1a Aug 2, 2015
@qfox
Copy link
Copy Markdown
Member

qfox commented Aug 2, 2015

Sorry for delay, merged! Thanks!

@weekens
Copy link
Copy Markdown
Contributor Author

weekens commented Aug 3, 2015

@zxqfox , didn't get, where is typo? And why did you call these 2 cases 'should not report valid jsdoc for function' and 'should not report valid jsdoc for method'? They ARE valid. And the validator should NOT report INVALID JSDoc.

If you'd like to call these 2 cases the way you do, it is inconsistent with https://github.com/jscs-dev/jscs-jsdoc/blob/master/test/lib/rules/validate-jsdoc/require-param-description.js, where the cases are called 'should not report invalid jsdoc for method' and 'should not report invalid jsdoc for function', respectively.

Please, explain your changes!

@qfox
Copy link
Copy Markdown
Member

qfox commented Aug 3, 2015

@weekens Double negation is a dirty thing, In JSCS we trying to avoid them and to put to case titles what they contains. So if there is valid jsdoc I prefer to put there "valid jsdoc". If there are no errors: better to put there "should not report". An so on. ;-)

You should look at https://github.com/jscs-dev/node-jscs/blob/master/lib/rules/require-multiple-var-decl.js — what this rule means? ;-) "Multiple var statements" or "Multiple variables"? ;-)

@qfox
Copy link
Copy Markdown
Member

qfox commented Aug 3, 2015

@weekens About require-param-description — Seems like need to fix that. ;-(

@weekens
Copy link
Copy Markdown
Contributor Author

weekens commented Aug 3, 2015

@zxqfox, I see your point. IMHO, if we want to put 'should not report' in front, then 'should not report errors for valid jsdoc for function' seems a better alternative. But anyway, thanks for explanation! Test case names is not really a big deal, pick the ones you're most comfortable with.

@qfox
Copy link
Copy Markdown
Member

qfox commented Aug 3, 2015

@weekens Yeah, nice suggestion. ;-) Thanks again!

jdlrobson pushed a commit to jdlrobson/jscs-jsdoc that referenced this pull request Aug 21, 2015
Added requireReturnDescription rule implementation, tests, and documentation.

Fixes jscs-dev#122
Closes jscs-devgh-125
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.

3 participants