Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29775602: Noissue - Update JSDoc to mark optionals in lib/filterClasses.js (Closed)

Created:
May 9, 2018, 4:52 p.m. by Manish Jethani
Modified:
May 11, 2018, 12:06 p.m.
Reviewers:
kzar
CC:
sergei
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Noissue - Update JSDoc to mark optionals in lib/filterClasses.js

Patch Set 1 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -29 lines) Patch
M lib/filterClasses.js View 9 chunks +28 lines, -29 lines 5 comments Download

Messages

Total messages: 6
Manish Jethani
May 9, 2018, 4:52 p.m. (2018-05-09 16:52:53 UTC) #1
Manish Jethani
Patch Set 1 I find it useful to know if something is optional, and the ...
May 9, 2018, 4:56 p.m. (2018-05-09 16:56:08 UTC) #2
kzar
https://codereview.adblockplus.org/29775602/diff/29775603/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29775602/diff/29775603/lib/filterClasses.js#newcode995 lib/filterClasses.js:995: * @type {string} On 2018/05/09 16:56:08, Manish Jethani wrote: ...
May 10, 2018, 11:06 a.m. (2018-05-10 11:06:25 UTC) #3
Manish Jethani
https://codereview.adblockplus.org/29775602/diff/29775603/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29775602/diff/29775603/lib/filterClasses.js#newcode995 lib/filterClasses.js:995: * @type {string} On 2018/05/10 11:06:25, kzar wrote: > ...
May 11, 2018, 3:20 a.m. (2018-05-11 03:20:59 UTC) #4
Manish Jethani
https://codereview.adblockplus.org/29775602/diff/29775603/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29775602/diff/29775603/lib/filterClasses.js#newcode995 lib/filterClasses.js:995: * @type {string} On 2018/05/11 03:20:57, Manish Jethani wrote: ...
May 11, 2018, 3:26 a.m. (2018-05-11 03:26:52 UTC) #5
kzar
May 11, 2018, 11:18 a.m. (2018-05-11 11:18:28 UTC) #6
On 2018/05/11 03:20:59, Manish Jethani wrote:
> So ElemHideBase is an abstract class, we don't create instances of it.
Assuming
> that this documentation here is going to be inherited by the derived classes,
> then it is true that selector will never be null.

Yea fair enough, I think your change is an improvement and like you say we might
be switching to proper ES6 classes in the future anyway. LGTM.

> I don't know if we're actually generating the HTML documentation out of this

We do, you can type `./build.py docs /some/path` to generate them yourself, or
take a look at them online at https://adblockplus.org/jsdoc/adblockpluscore.

> ...for now at least in the source code I hope that the JSDoc comments reflect
how things are.

Yea me too, thanks for taking the time to fix these things you notice.

Powered by Google App Engine
This is Rietveld