Closed Bug 636319 Opened 9 years ago Closed 6 years ago

Enable SDK developers to include API documentation in source

Categories

(Add-on SDK Graveyard :: Documentation, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: wbamberg, Assigned: wbamberg)

References

Details

Attachments

(1 file)

It should be possible for developers to document module in source, rather than forcing them to use separate Markdown files for API reference docs.

The idea is to use the 'docstract' tool: https://github.com/lloyd/docstract to parse specially formatted JavaScript comments into JSON.

The change should be done in such a way that docs can be moved over piecemeal from the MD files to the source. Also, people might choose to keep the separate MD files for module overview material, and only adding the detailed API reference into to source.

So the plan is something like this. 

For any given module:

- if an MD file exists, parse it. 
    - if it contains any plain Markdown, then consider this to be the module overview material, and don't look at the JS file for the module overview
    - if it contains any API reference stuff using <api></api> tagging, then consider this to be the API reference material, and don't look at the JS file for that, either

- if there wasn't an MD file, or the MD file didn't contain any Markdown, or the MD file didn't contain any API reference doc, then look in the JS file for whatever the MD file did not include.
Assignee: nobody → wbamberg
Attachment #517060 - Flags: review?(warner-bugzilla)
Here's a patch that enables people to document APIs in source, using the syntax expected by docstract: https://github.com/lloyd/docstract.

Sorry, it's kind of a huge patch, but most of it is:

(1) + BeautifulSoup.py
(2) + docstract.py
(3) test code: I changed the test code quite extensively, and although it's much more verbose and less readable too, I think it's actually more effective like this.

The full list of files touched:

packages/api-utils/lib/cortex.js
python-lib/cuddlefish/BeautifulSoup.py
python-lib/cuddlefish/apiparser.py
python-lib/cuddlefish/apiparser_md.py
python-lib/cuddlefish/apirenderer.py
python-lib/cuddlefish/docstract.py
python-lib/cuddlefish/server.py
python-lib/cuddlefish/tests/static-files/docs/APIreference.html
python-lib/cuddlefish/tests/static-files/docs/APIsample.js
python-lib/cuddlefish/tests/static-files/docs/APIsample.md
python-lib/cuddlefish/tests/static-files/packages/aardvark/docs/all.md
python-lib/cuddlefish/tests/static-files/packages/aardvark/docs/api.md
python-lib/cuddlefish/tests/static-files/packages/aardvark/docs/desc.md
python-lib/cuddlefish/tests/static-files/packages/aardvark/docs/empty.md
python-lib/cuddlefish/tests/static-files/packages/aardvark/lib/all.js
python-lib/cuddlefish/tests/static-files/packages/aardvark/lib/api.js
python-lib/cuddlefish/tests/static-files/packages/aardvark/lib/desc.js
python-lib/cuddlefish/tests/static-files/packages/aardvark/lib/empty.js
python-lib/cuddlefish/tests/test_apiparser.py
python-lib/cuddlefish/tests/test_apiparser_md.py
python-lib/cuddlefish/tests/test_apirenderer.py
python-lib/cuddlefish/tests/test_webdocs.py
python-lib/cuddlefish/webdocs.py
static-files/md/dev-guide/module-development/xpi.md

The idea is that it will work with the current method of documenting everything with <api></api> tags in MD files, and will continue to work as and when we migrate docs into the JS. To do this it uses these rules:

- a module's doc consists of two sorts of things: the general description, examples and so on, which is expected to be plain Markdown, and the api reference part, which uses a particular structured syntax (<api></api> in the Markdown style, /** @function */ in the JS in-source style)

- if there is a <module>.md file at all, parse it looking for module description and API reference docs. If there is a <module>.md file (even if it's empty), then it supplies the general description, and we don't look in <module>.js for a general description.

- if there isn't a <module>.md file, or if the <module>.md file exists but does not contain any <api></api> content, then parse the <module>.js file using docstract.

- if <module>.md didn't exist, use the general description from <module>.js

- if <module>.md didn't contain any api reference doc, use the api reference doc from <module>.js

So if we want to split the docs, and have the general description in separate MD files and the API reference in source, we can do that, and still have general descriptive text in source without the docs sytem getting confused. If we want to move everything into source we can do that too, by just deleting the MD files.

Just with the current docs you can see it working as it now picks up some API reference docs from cortex.js (though I had to tweak them a little).

A few things:

(1) the current code distinguishes between documented and undocumented modules, only listing documented modules in the sidebar and package pages. The idea is that lots of api-utils modules are just not interesting to developers (in particular the e10s ones), so listing them is not helpful. It distinguishes based on the existence of a <module>.md file: if this doesn't exist, the module's considered undocumented. But if we move docs entirely into the source, this obviously won't work any more.

I'm not sure what the right thing to do here is. What feels like the right thing would be to be able to indicate in some piece of metadata whether the module is 'internal' or not. But there's no obvious place to put this. An alternative could perhaps be to include internal modules in a separate subdirectory.

(2) you can't, yet, do all the things with the docstract syntax that you can with the MD syntax. Most notably you can't nest objects inside other objects, as we do with the components of `options` parameters. But that shouldn't stop this patch from landing, as it won't invalidate any of this work. We will be able to add some more tests when it happens. 

Less notably, the output of docstract doesn't set a 'name' attribute for constructors: this doesn't limit it because the name is always the same as the class name, but means we dont test constructor JSON for equality. Although it's redundant, it would be convenient, I think, if the ctor included a 'name' property, since everything else has one.

(3) when applying this patch git will complain about a bunch of trailing whitespaces. These are from the doctest inside 'xpi.md'. For some reason I had to regenerate all that output from the command prompt with ttrailing whitespace. I was expecting to have to update the doctest, but don't understand why it wanted trailing WS.
It's also at: https://github.com/wbamberg/jetpack-sdk/commits/636319, if that's an easier way to review it.
Attachment #517060 - Flags: review?(adw)
With this patch applied, when I run cfx testcfx with Python 2.5.1 I get this, a warning followed by an error:

(addon-sdk)09:13:18 ~/addon-sdk$ cfx testcfx
/Users/adw/addon-sdk/python-lib/cuddlefish/docstract.py:326: Warning: 'with' will become a reserved keyword in Python 2.6
Traceback (most recent call last):
  File "/Users/adw/addon-sdk/bin/cfx", line 29, in <module>
    cuddlefish.run()
  File "/Users/adw/addon-sdk/python-lib/cuddlefish/__init__.py", line 443, in run
    test_cfx(env_root, options.verbose)
  File "/Users/adw/addon-sdk/python-lib/cuddlefish/__init__.py", line 295, in test_cfx
    retval = cuddlefish.tests.run(verbose)
  File "/Users/adw/addon-sdk/python-lib/cuddlefish/tests/__init__.py", line 55, in run
    tests = get_tests()
  File "/Users/adw/addon-sdk/python-lib/cuddlefish/tests/__init__.py", line 21, in get_tests
    module = __import__(full_name, fromlist=[package.__name__])
  File "/Users/adw/addon-sdk/python-lib/cuddlefish/apiparser.py", line 1, in <module>
    import os, sys, apiparser_md, docstract
  File "/Users/adw/addon-sdk/python-lib/cuddlefish/docstract.py", line 326
    with open(filename, "r") as f:
            ^
SyntaxError: invalid syntax

When I run with Python 2.6.5, cfx testcfx passes all tests as expected with no warnings.

Does the SDK still support Python 2.5?  Are we willing to drop 2.5 for this and therefore support only 2.6?  Should we patch docstract?  I wish the SDK were becoming less reliant on Python and its many versions rather than more.
I'm not really comfortable reviewing such a large patch to a part of the SDK I don't know much about.  I'll keep looking at it and offering comments, but Brian and nobody but Brian needs to have final review on this.

python-lib/cuddlefish is getting cluttered.  A "docs" directory is there already.  I think docstract and Beautiful Soup and all other doc scripts new and existing should live there.
That's reasonable. Thanks for looking anyway.

> I wish the SDK were
> becoming less reliant on Python and its many versions rather than more.

I agree, actually. There's quite a lot of code in there now that exists only to generate docs. Should we think about pre-generating docs and shipping them as static HTML in the SDK? Or not shipping them at all and just hosting them online?
In other words, drop the doc server the idea of per-package docs.  I can hear Atul sighing.

That's a sufficient solution but not a necessary one to my concern, which is Python -- the fickleness between versions, requiring people to install it to use the SDK, the installation problems people report on the mailing list.

Ideally our doc system would be a collection of SDK modules.  Ideally cfx would be hosted in the browser.  Irakli and Alex have talked about doing that, and I recall Atul mentioning it too a long time ago.

We need to release SDK 1.0 ASAP and we have to live with our choices, but if I were king we'd stop writing new Python code today, except to maintain cfx, in preparation for moving to JS, the browser, and the SDK itself where possible.
(In reply to comment #7)
> In other words, drop the doc server the idea of per-package docs.

Drop the doc server *and* the idea of per-package docs, sorry.

(In reply to comment #2)
> particular the e10s ones), so listing them is not helpful. It distinguishes
> based on the existence of a <module>.md file: if this doesn't exist, the
> module's considered undocumented. But if we move docs entirely into the source,
> this obviously won't work any more.

If the module has no Markdown and docstract says it has no in-source docs, isn't that undocumented?

> I'm not sure what the right thing to do here is. What feels like the right
> thing would be to be able to indicate in some piece of metadata whether the
> module is 'internal' or not. But there's no obvious place to put this. An
> alternative could perhaps be to include internal modules in a separate
> subdirectory.

We really need to have a come-to-Jesus moment on internal vs. public.  (By "we" I guess I mean Myk.)  Using packages to distinguish the two is the right way to go; adding metadata is not.  We're kind of doing it now with api-utils and addon-kit, but it doesn't work when several stable and important APIs live in api-utils.
> 
> If the module has no Markdown and docstract says it has no in-source docs,
> isn't that undocumented?
> 

OK. I think I must have been thinking that developers might want to include /** */ style comments, but not have them picked up by the SDK docs system, but I suppose that doesn't make much sense.
Code comments forthcoming, but:

(In reply to comment #2)
> (2) you can't, yet, do all the things with the docstract syntax that you can
> with the MD syntax. Most notably you can't nest objects inside other objects,
> as we do with the components of `options` parameters.

Does this mean we're setting ourselves up to patch docstract to do things we need it to do?  Or more likely I guess, adapt our conventions to fit docstract (or any other in-source parser we'd end up using)?

Man, reading this patch makes me wonder whether we're making an add-on development product or a doc extraction and formatting product.
Comment on attachment 517060 [details] [diff] [review]
Integrate docstract into SDK's docs system

I'm going to play devil's advocate and suggest that if we're supporting in-source docs, it would be better to drop apidoc entirely than to support both.  That would mean more work upfront, converting all docs to in-source, but we want to do that anyway, and ultimately it means less code, less maintenance, fewer moving parts, fewer tests.

>diff --git a/python-lib/cuddlefish/apiparser.py b/python-lib/cuddlefish/apiparser.py

> if __name__ == "__main__":
>     json = False
>     if sys.argv[1] == "--json":
>         json = True
>         del sys.argv[1]
>-    docs_text = open(sys.argv[1]).read()
>-    docs_parsed = list(parse_hunks(docs_text))
>+    docs_parsed = get_api_json(sys.argv[1], sys.argv[2])
>     if json:
>         import simplejson
>-        print simplejson.dumps(docs_parsed, indent=2)
>-    else:
>-        TestRenderer().render_docs(docs_parsed)
>+        print simplejson.dumps(docs_parsed, indent = 2)

Why is the else clause removed?  (I don't know enough about this script's purpose.)

>diff --git a/python-lib/cuddlefish/apiparser_md.py b/python-lib/cuddlefish/apiparser_md.py

>+class APIParser:
>+    def parse(self, lines, lineno):
>+        api = {"line_number": lineno + 1}
>+# assign the name from the first line, of the form "<api name="API_NAME">"
>+        title_line = lines[lineno].rstrip("\n")

I know you didn't write these lines in this patch, but please fix the comment indentation here and elsewhere.

>+def parseTypeLine(line, lineno):

I know you didn't name this function in this patch, but other functions in this file and the doc scripts generally use underscores instead of camel case.

>+def extract(text):

Curious why you removed/rewrote parse_hunks -- it looks like the purpose of this new function is the same.

>+    lines = text.splitlines(True)
>+    line_number = 0;
>+    description = ""
>+    module_json = {"version" : VERSION, \
>+                   "classes" : [], \
>+                   "functions" : [], \
>+                   "properties" : [], \
>+                   "desc" : ""}
>+    while line_number < len(lines):
>+        line = lines[line_number]
>+        if line.startswith("<api"):
>+            break
>+        module_json["desc"] += lines[line_number]
>+        line_number = line_number + 1
>+    while line_number < len(lines):
>+        if not lines[line_number].startswith("<api"):
>+            line_number = line_number + 1
>+            continue

Is this if-statement necessary?  If the line does not start with "<api", then the first while-loop would not have ended until line_number becomes >= len(lines), which means the second while-loop would never have been entered.

>+        api_item, api_type, line_number = APIParser().parse(lines, line_number)
>+        if api_type == "class":
>+            module_json["classes"].append(api_item)
>+        elif api_type == "function":
>+            module_json["functions"].append(api_item)
>+        elif api_type == "method":
>+            module_json["functions"].append(api_item)
>+        elif api_type == "property":
>+            module_json["properties"].append(api_item)
>+    return module_json

extract() assumes that the module description must come before any <api> hunk.  Is that intentional (and do all our Markdown files conform)?  Sounds fine if so.  [Oh, I see the tests.  OK.]

Comments above all of these functions -- their purposes, parameters, and return values -- would be really helpful in understanding them.

>+def extractFromFile(path):

Underscores here too, although I realize you're probably trying to match docstract's function of the same name.

>+        md_text = open(path).read()
>+        data = extract(md_text)
>+        module_name = os.path.basename(path)
>+        dotLoc = module_name.rfind(".")
>+        if (dotLoc > 0):
>+            module_name = module_name[:dotLoc]
>+        if not "module" in data:
>+            data["module"] = module_name
>+        if not "filename" in data:
>+            data["filename"] = path
>+        return data

Too much indentation under this function.

>+if __name__ == "__main__":
>+    json = False
>+    if sys.argv[1] == "--json":
>+        json = True
>+        del sys.argv[1]
>+    docs_text = open(sys.argv[1]).read()
>+    docs_parsed = extract(docs_text)
>+    if json:
>+        import simplejson
>+        print simplejson.dumps(docs_parsed, indent=2)

This is doing pretty much the same thing as apiparser.py, no?  If so, please consolidate one into the other and use that everywhere we need an executable script.

>diff --git a/python-lib/cuddlefish/apirenderer.py b/python-lib/cuddlefish/apirenderer.py

>@@ -61,7 +60,8 @@ def indent(text_in):
>     for line in lines:
>         if (line.startswith('<div')):
>             text_out += ((' ' * indentation_depth) * indentation_level) + line
>-            indentation_level += 1
>+            if not '</div>' in line:
>+                indentation_level += 1

Nice catch.

>         else:
>             if (line.startswith('</div>')):
>                 indentation_level -= 1
>@@ -70,21 +70,25 @@ def indent(text_in):
> 
> def tag_wrap_id(text, classname, id, tag = 'div'):
>     return ''.join(['\n<'+ tag + ' id="', id, '" class="', \
>-                   classname, '">\n\n', text + '\n</' + tag +'>\n\n'])
>+                   classname, '">\n', text + '\n</' + tag +'>\n'])

I might be missing something here, but it's weird that these several lines that build strings use both `+` to concat and ''.join().

> def tag_wrap(text, classname, tag = 'div'):
>-    return ''.join(['\n<' + tag + ' class="', classname, '">\n\n', \
>-                   text, '\n</'+ tag + '>\n\n'])
>+    return ''.join(['\n<' + tag + ' class="', classname, '">', \
>+                   text, '\n</'+ tag + '>\n'])
>+
>+def tag_wrap_inline(text, classname, tag = 'div'):
>+    return ''.join(['\n<' + tag + ' class="', classname, '">', \
>+                   text, '</'+ tag + '>\n'])

Having a way to do "inline" strings of HTML is fine, but the only difference between these two functions is a single \n.  Instead of adding tag_wrap_inline, tag_wrap should take an extra parameter "inline" or something that defaults to False.

> def render_object_contents(json):
>-    ctors = json.get('constructors', None)
>-    text = render_comp_group(ctors, 'Constructors', Function_Doc)
>-    methods = json.get('methods', None)
>+    ctor = json.get('constructor', None)
>+    text = ''
>+    if ctor:
>+        text += render_comp_group([ctor], 'Constructors', Function_Doc)
>+    methods = json.get('functions', None)

You made "constructor" singular in the JSON object (but left the plural "Constructors" as the arg passed to render_comp_group) -- does that mean it's no longer possible to document overloaded constructors separately?  Is that because of docstract?  Not terribly important, just curious.

>@@ -210,7 +205,8 @@ def render_comp(component):
>     # a component is wrapped inside a single div marked 'API_COMPONENT'
>     # containing:
>     # 1) the component name, marked 'API_NAME'
>-    text = tag_wrap(component.render_name(), API_NAME, component.get_tag())
>+    text = tag_wrap_inline(component.render_name(), \
>+                           API_NAME, component.get_tag())

I don't think the trailing backslash is necessary, since the line ends in a comma...?  Could be wrong.  Below too.

> def render_api_reference(api_docs):
>-    if (len(api_docs) == 0):
>+    if (len(api_docs["classes"]) == 0) and \
>+       (len(api_docs["functions"]) == 0) and \
>+       (len(api_docs["properties"]) == 0):

This is the negation of the return value of api_json_exists() in apiparser.py.  You might ask yourself why you're duplicating code and probably call api_json_exists() here or whatever so you don't duplicate.

>diff --git a/python-lib/cuddlefish/tests/test_apirenderer.py b/python-lib/cuddlefish/tests/test_apirenderer.py

>+    def validate_html(self, html):
>+# we won't test everything in here, just the basic structure of the doc
>+        soup = BeautifulSoup.BeautifulSoup(html)
>+        self.assertEqual(soup.html.body.div["id"], "APIsample_module_api_docs")
>+        self.assertEqual(soup.html.body.div["class"], "module_api_docs")
>+        self.assertEqual(soup.html.body.div.h1.string, unicode("APIsample"))
>+        top_level_divs = soup.html.body.div("div", recursive = False)

Hmm, not a huge deal, but can you not use HTMLParser instead of bringing BeautifulSoup in to the tree for these few lines?
Attachment #517060 - Flags: review?(adw)
Duplicate of this bug: 569210
Attachment #517060 - Flags: review?(warner-bugzilla)
Thanks for your review on this Drew. I appreciate (and share) your worries about adding a lot more Python code into the SDK.

Given that (1) docstract doesn't yet support everything we need, (2) that there's only one more beta before 1.0 (and I would certainly want this change to be delivered in that beta, rather than first appearing in 1.0) , and (3) that this change doesn't add any direct benefit to SDK users in the short term, this is starting to feel to me like something we should revisit after 1.0. 

> > def render_object_contents(json):
> >-    ctors = json.get('constructors', None)
> >-    text = render_comp_group(ctors, 'Constructors', Function_Doc)
> >-    methods = json.get('methods', None)
> >+    ctor = json.get('constructor', None)
> >+    text = ''
> >+    if ctor:
> >+        text += render_comp_group([ctor], 'Constructors', Function_Doc)
> >+    methods = json.get('functions', None)
> 
> You made "constructor" singular in the JSON object (but left the plural
> "Constructors" as the arg passed to render_comp_group) -- does that mean it's
> no longer possible to document overloaded constructors separately?  Is that
> because of docstract?  Not terribly important, just curious.

This is my ignorance of JavaScript showing. In the original <api></api> syntax, you can have multiple constructors for a given object (although none of the SDK modules do). Thus the docs output has a heading "Constructors" along with "Methods" and "Properties".

In docstract you can't: if you have 2 ctors for a class only the second one is included (i.e. it overwrites the previous one). I had just assumed that was correct, but if having overloaded constructors is valid, then it's a docstract bug.

> Hmm, not a huge deal, but can you not use HTMLParser instead of bringing
> BeautifulSoup in to the tree for these few lines?

Yeah, I'll have a go at that. I will try to land these test changes anyway, as part of bug 617029.
Hi folks, I'm back from leave.

(In reply to comment #13)
> Given that (1) docstract doesn't yet support everything we need

Are there issues opened here for everything that's missing?
https://github.com/lloyd/docstract/issues
 
> In docstract you can't: if you have 2 ctors for a class only the second one is
> included (i.e. it overwrites the previous one). I had just assumed that was
> correct, but if having overloaded constructors is valid, then it's a docstract
> bug.

Opened an issue: https://github.com/lloyd/docstract/issues/9
(In reply to comment #14)
> Opened an issue: https://github.com/lloyd/docstract/issues/9

and closed that issue.  multiple constructors are now supported and the json output format is changed accordingly.
Hey Lloyd, welcome back.

> (In reply to comment #13)
> > Given that (1) docstract doesn't yet support everything we need
> 
> Are there issues opened here for everything that's missing?
> https://github.com/lloyd/docstract/issues

I think so. Personally I think a "name" attribute on constructors makes processing easier, although it is redundant.
OS: Mac OS X → All
Priority: -- → P3
Hardware: x86 → All
Target Milestone: --- → 1.0
(automatic reprioritization of 1.0 bugs)
Priority: P3 → P2
Target Milestone: 1.0 → 1.1
I'd suggest marking this as WONTFIX.

The patch attached here works well enough, but the points about increasing the complexity of the docs code, and the dependency on Python, are good ones.

In the fairly near future we certainly want to support localization of the docs and may want to integrate them with MDN. In either of those scenarios supporting in-source docs looks a lot more complicated, and they both seem to me more valuable than having in-source docs.
This bug is not about Python. That's an implementation choice/detail. This bug is about making it far easier for *developers* to document their code. Reducing the barrier to writing documentation reaps huge benefits, even if those initial docs need massaging by a professional.

The current system is much more difficult than allowing inline API documentation IMO.

If you WONTFIX this due to it's orientation towards a particular implementation, fine - rename it, close it, and we can open a new one. But IMO this bug as described by it's current summary should not be closed.
Maybe that's a separate bug but it seems that other projects ( jQuery ) use inline documentation and produce reasonable documentation from it. 

Separate docs is different than the norm, which reduces ther SDK's ability to integrate with other dev tools / IDEs / Editors.
(In reply to Will Bamberg [:wbamberg] from comment #18)
> In the fairly near future we certainly want to support localization of the
> docs and may want to integrate them with MDN. In either of those scenarios
> supporting in-source docs looks a lot more complicated, and they both seem
> to me more valuable than having in-source docs.

a) concerning localization of the PROGRAMMER documentation, I will believe when I see it. If you think it will be something on the level with the current localization of MDN (see for an example https://developer.mozilla.org/es/DOM/window ... there are whole two methods' description linked from the page), then I think we should not bother and expect (as everyone everywhere expects until now) that programmers need to learn English. BTW, before you accuse me of anything, I am a Czech and English is my second (or third, after Russian which I mostly forgot) language. I would bother about localized documentation when it happens and not stall the progress based on it.
b) Do we have some numbers how many Add-on SDK projects on github/Add-on Builder (i.e., outside of the official Add-on SDK tree) have ANY documentation (i.e., they have docs/ directory)? Just by random browsing through few projects on Add-on Builder (and searching through few projects on github) I haven't found any (I found couple using JavaDoc-style in-code comments despite this being unsupported).
Isn't it the time to call this out-of-code documentation complete failure? BTW, which substantial FLOSS projects at all (outisde of MoFo projects) use such out-of-the-code API documentation? I see JavaDoc/JSDoc/Doxygen/etc. everywhere (my colleagues around Gnome 3 are now even generating language bindings out of the comments in the code), but I cannot recall now any project which would keep API documentation outside of the code. I may be wrong of course in this recollection.
(In reply to Matej Cepl from comment #21)
> b) Do we have some numbers how many Add-on SDK projects on github/Add-on
> Builder (i.e., outside of the official Add-on SDK tree) have ANY
> documentation (i.e., they have docs/ directory)? Just by random browsing
> through few projects on Add-on Builder (and searching through few projects
> on github) I haven't found any (I found couple using JavaDoc-style in-code
> comments despite this being unsupported).

Sorry, my mistake, Add-on Builder (which I don't use myself) apparently doesn't have a way to have docs/ (or I missed it, because I haven't found any documentation for the Builder itself).
There is a patch attached to this bug that makes in-source docs possible via Lloyd's awesome docstract.py. It works and everything. It wouldn't be too hard to un-rot it and apply it. But the feedback I've seen in this bug is that it's not the best approach, because (1) it adds a lot of new code, and (2) that code is in Python.

Addressing (1) is difficult: it seems to me that an in-source doc parser is a fairly complex program. I'm not sure which existing parsers might be suitable, and not drag in undesirable dependencies of their own, and it's difficult to see how such a thing could not add a substantial amount of complexity to the docs system. Especially since we want to document quite abstract things, like events, that are represented in syntactically very diverse ways in the various event emitting modules. If anyone has any suggestions to help with this I'd be really happy to hear them.

As for (2), it's hard for me to see how we could avoid using Python for this in the current design of cfx. When, and if, JS-cfx happens, it's a different story. But I don't know when JS-cfx is going to happen.

So I'm not sure what the best way forward is with this bug, right now. Maybe it's such an important thing that it's worth the Python, and the complexity. Maybe someone who's a better developer than me can help implement something more acceptable, or at least provide some helpful suggestions. Maybe we should wait until JS-cfx comes along.

Finally, it seems to me that the absence of docs for third party SDK modules is probably not just because the SDK docs system doesn't enable in source docs. The docs system relies on cfx docs finding your markdown and building the HTML dynamically. It doesn't fit very well with developers who might want to generate static HTML so a user can browse the docs online before installing the package. Addressing this is part of a bigger project to address the needs of third party module developers, and it seems to me that as Jetpack has, so far, mostly focused on add-on developers.
(In reply to Dietrich Ayala (:dietrich) from comment #19)

> If you WONTFIX this due to it's orientation towards a particular
> implementation, fine - rename it, close it, and we can open a new one. But
> IMO this bug as described by it's current summary should not be closed.

I used to think stagnant bugs for desired features were useful, as reminders of what we eventually plan to do.  But over the years, I've noticed that such bugs rarely help.

Usually, by the time someone gets around to implementing the feature, memory of the bug has faded, and the implementer files a new one, with the old one eventually getting duped to it (or vice-versa, but usually the new one has more valuable history by the time the duping takes place).

Alternately, the implementer reuses the bug, but the feature the implementer eventually implements is different enough that the original bug's summary and description doesn't really address it, and it would have been better to have worked in a new bug.

Also, these days we're moving to feature pages for tracking major new features like this (although we continue to track minor enhancements and bugs in Bugzilla).

So if this is not a priority (which isn't yet clear), and we don't intend to implement it in the foreseeable future, then we are better off closing this bug than leaving it here to rot.  If you then want to see it implemented, the right response is to create a feature page and convince the Jetpack product manager to prioritize it (or implement it yourself).


(In reply to Matej Cepl from comment #21)

> Isn't it the time to call this out-of-code documentation complete failure?

No.  To the contrary, our documentation process, which includes both developer-contributed documentation and tech writing by a dedicated tech writer, has been quite successful at getting high-quality documentation of the Add-on SDK's features and APIs into the hands of addon developers.

That doesn't mean our docs are perfect--they aren't--or couldn't be better--they could.  Nor does it mean that this feature wouldn't help improve them--it might.  But it's gross hyperbole and quite unfair to the dedicated folks who have put a ton of time and energy into the SDK docs to suggest this.
(In reply to Myk Melez [:myk] from comment #24)
> But it's gross hyperbole and quite unfair to the
> dedicated folks who have put a ton of time and energy into the SDK docs to
> suggest this.

Talking about unfair, I haven't mentioned in a word quality of already generated SDK documentation, which is truly outstanding. Whole this bug is about the third party documentation and how to make documenting process for the third party developers more simple.

If you don't care about that, just say so, and WONTFIX this bug, but please don't put words (and especially offenses) into my mouth.

Thank you
(In reply to Matej Cepl from comment #25)
> Talking about unfair, I haven't mentioned in a word quality of already
> generated SDK documentation, which is truly outstanding. Whole this bug is
> about the third party documentation and how to make documenting process for
> the third party developers more simple.

Actually, this bug is about making the documentation process simpler for SDK developers, as noted in the summary.


> If you don't care about that, just say so, and WONTFIX this bug, but please
> don't put words (and especially offenses) into my mouth.

I didn't put any words into your mouth.  But I did misunderstand what you said, for which I apologize.

My answer is the same, though: no, it is not time to call out-of-code documentation for third-party API developers a complete failure, or even a partial one, since we haven't yet focused on the needs of those developers, and we don't yet know what features we would develop for that audience, and how successful they would be.

We do care about their needs, though, and if we close this bug, it will only reflect our best judgement about the value of this feature relative to its cost, not a lack of caring on our part.
(Pushing all open bugs to the --- milestone for the new triage system)
Target Milestone: 1.1 → ---
Now the docs are on MDN, this is not going to happen.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.