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 the API documentation mess #774

Closed
marcelstoer opened this issue Nov 18, 2015 · 52 comments
Closed

Fix the API documentation mess #774

marcelstoer opened this issue Nov 18, 2015 · 52 comments
Assignees

Comments

@marcelstoer
Copy link
Member

I want to drop the Wiki documentation and move the content to the repository so we can serve it with Read The Docs. See http://read-the-docs.readthedocs.org/en/latest/features.html for details. This would solve two major pain points:

  • Documentation not in line with the code

    If the documentation is moved to the code repository we can verify with every commit/PR that the documentation is updated accordingly. Furthermore, the documentation is merged automatically with the code if it moves from branch X to Y. The human-readable docs are built automatically with every commit.

  • L10n

    Read The Docs (and similar solutions) offer L10n to easily serve the documentation in multiple languages. This would replace the inconsistent Wiki pages for non-EN languages.

@jmattsson
Copy link
Member

I've never used Read The Docs, but as I've mentioned elsewhere I'm all in favour of switching to something like this. I had a look through the docs but I didn't see much in terms of what mark-up/down/sideways could/would/should be used. I'm familiar with JavaDoc / Doxygen syntax which works pretty well for documenting code. For user-facing docs it's probably not as appropriate (though the easy API cross-referencing is most handy).

Anyway, I guess what I'm trying to say is that I'd be happy to give anything a go as long as it seems reasonable, and especially if you have experience working with this particular flavour and can set a good example for the rest of us :)

@marcelstoer
Copy link
Member Author

I had a look through the docs but I didn't see much in terms of what mark-up/down/sideways could/would/should be used.

It supports reStructuredText and Markdown.

@jorgempy
Copy link
Contributor

+1

El mié., 18 de nov. de 2015 a la(s) 8:39 a. m., Marcel Stör <
[email protected]> escribió:

I had a look through the docs but I didn't see much in terms of what
mark-up/down/sideways could/would/should be used.

It supports reStructuredText and Markdown.


Reply to this email directly or view it on GitHub
#774 (comment)
.

@dzcpy
Copy link

dzcpy commented Nov 19, 2015

+2

@TerryE
Copy link
Collaborator

TerryE commented Nov 20, 2015

I've never come across Read The Docs before but I have used and am familiar with Doxygen both for end-user and programming documentation, so it would get my vote simply on the grounds that it is one less system to learn, but what the heck if someone sets up a usable system then I'll use it. eLua uses another system -- can't recall which off the top of my head.

@marcelstoer
Copy link
Member Author

I just found out that the SmartArduino/DOIT/MXCHIP.INC folks do a pretty good job with the introductory documentation for WiFiMCU at https://www.gitbook.com/book/fineshang/wifimcu-based-on-emw3165-user-manual/details. As you can see they're on GitBook which is veeery similar to RTD.

Others are in a similar situation as we are: https://civicrm.org/node/3268, http://j.mp/1YqisPD, http://wp.me/p2YeUz-1sG.

@jmattsson
Copy link
Member

Gitbook looks as viable as RTD to my untrained eyes.

What I really meant to ask in my previous comment was about schemas and/or templates. With e.g. Doxygen you get a bunch of semantic keywords/commands (\param, \returns, \see, etc) which not only have associated styling, but can also do Useful Things(tm) such as automatic cross-linking to other pages. In terms of templates, I'm thinking along the pre-ordained each-function-shalt-have-intro-and-param-plus-return-descriptions-at-minimum, or what we currently have on the wiki for example. Whenever you start documenting a new function you have a clear template of what must be included at minimum.

Would we get anything along those lines with either of these approaches? Both seem to want to me sign up before they tell me any particulars grumble. Not a deal-killer if they don't offer these things, but anything that makes it easy to write good, consistent documentation is highly valued in my opinion.

@TerryE
Copy link
Collaborator

TerryE commented Nov 24, 2015

Johny, +1 to your comment, but we are of similar generation in terms of our development experience. The young guys might disagree. My feeling is that there are many types of documentation, and its hard to do one cap fits all. Where things like Doxygen is particularly strong is documenting code. What we are talking about here is

  • Documenting the runtime API that the libraries implement.
  • The occasional whitepaper on more complex issues such as LTR, LCD and your flash exception handler.
  • Tutorials and code examples
  • FAQs

The first of these needs to be locked to the source IMO, so that if you change the API then you change the documentation for it in the same file. But again that's just my POV.

@jmattsson
Copy link
Member

I'm quite happy to let Marcel pick something he thinks is a good fit, and point me at instructions on how to use it. In the meantime I'm just throwing my $0.02 pieces into the thread.

We have a saying in Sweden which translates as "Much chatter, no hockey", meaning to just get on with the action and not worry about whether the call was the best or not. As long as we're doing things which are better than they used to be, we're moving in the right direction. Let's not overthink things too much.

Oh, and the L10n support of RTD seemed very applicable to this project, and I don't see that listed on GitBooks.

@zhujunsan
Copy link
Contributor

I mentioned to use RTD as the doc to the original nodemcu developer like a year ago, and I tried it out myself and created
https://readthedocs.org/projects/nodemcu-firmware/ which connected to my own fork. But sense than I haven't updated it. @marcelstoer emailed me today and I deleted my RTD project so that it can be pointing to this repo.

I think RTD is a very good tool to maintain a readable and user friendly document and I'll help if the decision is made to move the doc to RTD :)

@jmattsson
Copy link
Member

Since I promised I'd spend some of my leave working on the docs, I took the liberty of setting up a new (temporary) branch here called newdocs. This branch contains @marcelstoer 's work on getting us up and going with RTD, and I've transferred/added documentation for ~10 modules so far. I've been focusing on the ones which I actually know something about, so that I've been able to add a little bit of useful information here and there while copying/transcribing.

RTD itself is still configured to look at Marcel's repo, so there'll be nothing visible there just yet.

Unless anyone objects to the approach of getting the docs prepped in a sidebranch, I'd invite people to join in the fun of getting the docs across ;P

@marcelstoer
Copy link
Member Author

setting up a new (temporary) branch here called newdocs

You beat me to it, thanks. I had planned that for tonight. I rushed to remove it from the cloud builder.

RTD itself is still configured to look at Marcel's repo

I'll fix that in the next hours.

Unless anyone objects to the approach of getting the docs prepped in a sidebranch

Would surprise me as I had proposed this a month ago and no one has objected since 😉

@marcelstoer
Copy link
Member Author

I spent some time in the past days on a consistent and cohesive presentation of the content. I just committed some tweaks to the theme and an overhaul of the node, adc, and bit module docs. Here's a list of issues I found and what I changed:

  • sorted all functions alphabetically (grep '## ' xxx.md | sort)
  • removed : after parameter name
  • only kept . at end of parameter and return value description if complete sentence (hardly ever)
  • nil is nil consistenly
  • no parameters -> 'none' instead of nil, this makes a lot more sense to me but I wonder how that feels for English native speakers
  • return type only if not mentioned in description or obvious (e.g. true/false), if so then added after return description
  • all cross-referenced functions mentioned in description or "See also" have a hyperlink
  • if there's an "Example" (see node.md) then I removed the "Syntax" because it's totally superfluous
  • removed the manual horizontal line between functions but added header bottom border instead (inspired by GitHub)
  • removed any extra indentation in the example code

If you want to compare before/after please compare node, adc, and bit with the other modules.

I'll continue updating the others for consistency unless there's objection from the collaborators.

@devsaurus
Copy link
Member

Ack with all your points except

if there's an "Example" (see node.md) then I removed the "Syntax" because it's totally superfluous

I'd keep the "Syntax" for formal reasons. Grammar aspects like nesting and optional parameters should be shown here in an exhaustive way. IMO better remove "Example" in case it's a trivial one.

@marcelstoer
Copy link
Member Author

Thanks for checking.

Grammar aspects like nesting and optional parameters should be shown here in an exhaustive way. IMO better remove "Example" in case it's a trivial one.

Ok, understood. Makes sense.

@jmattsson
Copy link
Member

Other than what @devsaurus already pointed out, sounds good to me!

I'm on the fence when it comes to trivial examples. One the one hand they're largely redundant, but on the other hand they might be nice for someone new to NodeMCU. Multiple-return (e.g. foo, _, bar = baz()) examples should always stay imho - many languages don't support them so usage might not be obvious otherwise.

I'm not sure I'll have any more time this week to transfer docs, but I'll see how I go. More hands appreciated, wink-wink-nudge-nugde ;)

@devsaurus
Copy link
Member

Is it OK if I hijack the Module origin table and add a column "Transfer status" to it? We'd see what's TBD and also if someone else is currently busy with a module. Just to avoid merge conflicts / double effort while the big chunks are being moved.
The column should be deleted afterwards, it's probably not useful to capture ongoing maintenance.

@TerryE
Copy link
Collaborator

TerryE commented Jan 10, 2016

This is looking really good, but just looking at your example link, I really need to add another issue which is to do a complete API review of return statuses. If a call can errors, then it should either throw an error or return an error status. In the former case the documentation should say that it throws an error on error. (This struck me when reading node.compile() which just says "returns nil".)

@marcelstoer
Copy link
Member Author

Is it OK if I hijack the Module origin table and add a column "Transfer status" to it?

+1 if you call it "Doc transfer status" 😉. Have been thinking about that last night as well.

@devsaurus
Copy link
Member

if you call it "Doc transfer status"

Done.

Apparently, we're missing API docs for a couple of modules (while some come with a README.md example at least):

Are the contributors still planning to provide an API description? Either in the wiki nodemcu_api_en or - preferably - as a new file in newdocs branch.

@christakahashi
Copy link
Contributor

HX711 docs written. Pull req #919

@nodemcu
Copy link
Collaborator

nodemcu commented Jan 11, 2016

Thanks @devsaurus, I will do the coap docs ASAP when I get some time.

@devsaurus
Copy link
Member

@christakahashi @nodemcu thanks a lot!

@marcelstoer
Copy link
Member Author

Only 4 more modules to transfer, way to go 😉

Are the contributors still planning to provide an API description?

At some point (how about now?) we might wanna make this a prerequisite for "accepting" a module.

@marcelstoer
Copy link
Member Author

My vote is to keep it because I'd like to have single source we can link to if we send people to RTFM.

@kbeckmann kbeckmann mentioned this issue Jan 14, 2016
@kbeckmann
Copy link
Contributor

BMP085 docs done #936

@devsaurus
Copy link
Member

Thanks @kbeckmann!

@pjsg
Copy link
Member

pjsg commented Jan 15, 2016

I can certainly help with the FAQ. I get short periods of time when I can work on stuff at home. My day job is keeping me rather busy at the moment....

@TerryE
Copy link
Collaborator

TerryE commented Jan 15, 2016

I used to have that sort of problem. So I understand. Now I am retired, but an a bit occupied with building a house instead! It's mainly constructive feedback and comments that I will need -- your chance to get your own back now that our roles are reversed 😉

@TerryE
Copy link
Collaborator

TerryE commented Jan 15, 2016

I realise that it would add some work, but it would really enhance the readability of each documentation section if it were prefixed by a sort table of contents for example in the case of node, this would be as follows (note that the first column would be a MD anchor link to the relevant section of the documentation):

Method / Constant Short Description
node.bootreason() Returns the boot reason and extended reset info
node.chipid() Returns the ESP chip ID
node.compile() Compiles a Lua text file into Lua bytecode, and saves it as .lc file
node.dsleep() Enters deep sleep mode, wakes up when timed out
node.flashid() Returns the flash chip ID
node.heap() Returns the current available heap size in bytes
node.info() Returns NodeMCU version, chipid, flashid, flash size, flash mode, flash speed
node.input() Submits a string to the Lua interpreter. Similar to pcall(loadstring(str)), but without the single-line limitation
node.key() --deprecated Defines action to take on button press (on the old devkit 0.9), button connected to GPIO 16
node.led() --deprecated Sets the on/off time for the LED (on the old devkit 0.9), with the LED connected to GPIO16, multiplexed with [node.key()]
node.output() Redirects the Lua interpreter output to a callback function. Optionally also prints it to the serial console
node.readvdd33() --deprecated Moved to adc.readvdd33()
node.restart() Restarts the chip
node.restore() Restores system configuration to defaults. Erases all stored WiFi settings, and resets the "esp init data" to the defaults. This function is intended as a last-resort without having to reflash the ESP altogether
node.setcpufreq() Change the working CPU Frequency
node.stripdebug() Controls the amount of debug information kept during node.compile(), and allows removal of debug information from already compiled Lua code

We could also drop the rotable prefix, e.g. node. in this case. I also think that tabling the inline entries will significantly compact the layout and make it more readable, e.g

node.compile()

node.compile() Compiles a Lua text file into Lua bytecode, and saves it as .lc file.
Syntax node.compile("file.lua")
Parameters filename name of Lua text file
Returns nil
Example
file.open("hello.lua","w+")
file.writeline([[print("hello nodemcu")]])
file.writeline([[print(node.heap())]])
file.close()

node.compile("hello.lua")
dofile("hello.lua")
dofile("hello.lc")

@marcelstoer
Copy link
Member Author

...enhance the readability of each documentation section if it were prefixed by a sort table of contents...

True, but that should be generated rather than manually maintained. I don't think MkDocs has support or extensions for that but I'll spend some time to find alternatives.

I also think that tabling the inline entries will significantly compact the layout and make it more readable.

Terry, where were you when we discussed the new layout a month ago 😉. Except for 1-wire everything has since been transferred and corrected (see status on Module-origin). Hell of a task... Anyone is welcome to change it again but I'm done with it.
This is not to say that your proposal didn't have its charm. You might even be able to batch-convert all .md files with a script since the structure should be pretty consistent now.

@marcelstoer
Copy link
Member Author

I don't think MkDocs has support or extensions for that but I'll spend some time to find alternatives.

Turned out to be a correct assumption. Since we can't directly tinker with the static doc generation process I resorted once again to JavaScript. The TOC is now generated on each API doc page on the fly in the browser (you might have to hit shift-reload to get the updated .js file).

@TerryE
Copy link
Collaborator

TerryE commented Jan 15, 2016

Marcel, I was up to eyes in my bloody house.

We can only do stepwise improvement. Yes, your exercise is a major improvement, but I made the mistake of using the Github viewer rather than RTD. The RTD TOC is close enough, and and inline TOC embedding a short description is difficult to generate inline automatically. Also the RTD CSS is far better than the default MD one, so the RTD view is a major step forward compared to the Github MD renderer. Hwever at the moment, the only way that you can see this is online. What would be brilliant is a mechanism for dumping out the entire documentation to CHTML so that the developer always has this reference to hand.

@TerryE
Copy link
Collaborator

TerryE commented Jan 16, 2016

Marcel, I've just looked at your latest version of the RTD documentation. I think that this is really good. Thanks for your work

@devsaurus
Copy link
Member

@TerryE

Shall we then keep the FAQ in newdocs ... or would you prefer to have it removed for the time being?

That's the last gating topic before merging newdocs into dev. IMO #937 is a kind of 👎, isn't it?

@TerryE
Copy link
Collaborator

TerryE commented Jan 16, 2016

@devsaurus Arnim, I don't understand your thumbs down. Documentation can always be improved. My current unofficial FAQ contains a lot of extremely useful information. I think that what you've all done is a major improvement in the documentation base, and I don't really want to hold up the merge, so we could include the current FAQ. It's only a few hours work to convert it from dokuwiki markup to github MD, and Marcel has already done this as at 15 Dec to create the current faq.md. So my vote is 👍

What my #937 is really about is an admission that there are a couple of points where (after months of working and developing with the firmware and Lua) I now think that my advice is wrong, in particular my advice to avoid using upvals to store context, and to avoid coroutining. I now understand how the Lua core, the nodemcu additions and the SDK all interact and these are solid, so it is sensible to use these features in the appropriate use case.

There are also few area where I would restructure the advice to make it more readily understandable to programmers new to Lua and the esp8266, and there a quite a few extra topics that I'd like to add.

In hoisting this to an official FAQ, what I suggest is an approach a bit like a lot of release strategies work. We have a fairly dynamic "living" FAQ that we can work on collaboratively say in a wiki, but that we snapshot stable updates by my doing a PR to merge these into the dev into every 3-6 months, or after any major improvements. #937 is just an issue to collaborate on the next "catch-up".

@jmattsson
Copy link
Member

Hey @marcelstoer I like the TOC at the top, but the net module's Constants header shows up a bit strange. Could you have a look please?

@TerryE where would said HTML bundle go/come from? It shouldn't be part of the repo. It could potentially be built during CI, but then where would it go? Should the cloud builder include a link to said bundle? Should it be pushed as a release back to the github repo? Should the bundle only be a download link via RTD, for whatever is shown on that RTD branch?

Also @TerryE some of the links in the faq.md don't seem to show up properly on RTD.

@devsaurus
Copy link
Member

Also @TerryE some of the links in the faq.md don't seem to show up properly on RTD.

That's what I plan to fix right now. #937 was holding me back from spending the effort - thus my earlier request. Terry clarified this, so way to go... 😁.

@TerryE
Copy link
Collaborator

TerryE commented Jan 16, 2016

eLua generates its CHTML as part of the build, but I think that doing this is a pain since the build than requires a whole stack of Latex-style authoring components. I think that a pragmatic alternative is to have this HTML bundle as some form of downloadable resource, just like the sdk zip and the open-sdk tarball used in the Travis CI builds. Maybe include the link to this in the Docs header or useful links. That way developers who want to access the documentation on the laptop when out of network range can download the bundle and access it locally.

Arnim, if you do make any changes or corrections to the faq.md, then I'll diff these to track them and backload them into working version, so we don't repeat this issue on the next update. Thanks.

@jmattsson
Copy link
Member

I see that RTD has some sort of Downloads area - I wonder if we could drop the per-branch documentation there. I'd say that's for later down the track though - in the meantime you can just grab the repo and fire up your favourite markdown editor to peruse the docs locally.

@devsaurus
Copy link
Member

Terry, the links are now ported to Markdown syntax - no changes to the actual document contents. Someone will have to go through it again when importing updates as long as the FAQ master is maintained in a different ecosystem.

Marcel, no blocking points from my side anymore for merging into dev.

@TerryE
Copy link
Collaborator

TerryE commented Jan 16, 2016

Incidentally Arnim, what I feel that you need for collaborating and working on a markup syntax is (i) the ability to work on an individual section and (ii) to have an immediate preview before commit, (iii) preferably a wysiwyg editor. The github viewer/editor suck at all of these, and it also lacks some of the basic markup features that I'd really like to use such as indented paragraphs. But I guess that I'll just have to live with this. WP markup is a lot more flexible, IMO.

Whatever the markup syntax used for collaborating, I'll end up writing or porting some hack perl to do the conversion to md. That's what I've done in the past. I've used Perl for this type of work for 25 years so its gettting a bit late to change old habits 😆 but don't work this quick & dirty stuff won't go near this repo.

@marcelstoer
Copy link
Member Author

My $0.02 to the "HTML bundle" discussion:

  • IMO just a nice-to-have goodie, aren't developers always online? If you really really need it offline you can clone the repo (in advance...) and look at the .md files with the editor of your choice.
  • Should definitely not be part of our repository.
  • For Sphinx-based projects RTD generates a doc PDF with every build for offline use. I'm pretty sure it's just a matter of time until that becomes available for MkDocs projects like ours as well. Hence, sooner or later we'll get that for free.

A few minutes ago a put the finishing touches on the new docs. My plan for the immediate next steps is as follows:

  • transfer the last outstanding module (1-wire), wouldn't mind if someone beat me to it
  • merge newdocs back to dev Sunday night CET and remove it
  • remove the sections from the README that are covered in the docs
  • add a prominent link to the RTD instance to the README
  • empty https://github.com/nodemcu/nodemcu-firmware/wiki/nodemcu_api_en and put a link to the RTD instance there instead
  • contact the maintainers/contributors of the translated wiki API docs (all either outdated or sparse) and encourage them to update & transfer their content
  • close this issue Monday night CET the latest

@TerryE
Copy link
Collaborator

TerryE commented Jan 16, 2016

My $0.02 to the "HTML bundle" discussion:

  • IMO just a nice-to-have goodie, aren't developers always online? If you really really need it offline you can clone the repo (in advance...) and look at the .md files with the editor of your choice.

No. I don't think that this is a valid assumption, yet.

  • Should definitely not be part of our repository

No more than any derived product like object libraries or binaries. But there is a stronger argument for saying that the scripts to create them should be.

@jmattsson
Copy link
Member

No. I don't think that this is a valid assumption, yet.

I'm not sure if you're speaking more philosophically here or are actually arguing against "releasing" newdocs until we also provide an offline bundle. If the former I agree, but if the latter I strongly disagree. newdocs is a huge improvement over the existing situation (which also doesn't provide offline access without cloning the separate wiki repo). The wiki API docs are incomplete and out-of-date, and we're doing everyone a disservice the longer we have them around.

Since it also seems likely that someone else will provide the necessary glue for also getting a PDF generated, I'm in favour of waiting for that and then getting that into the RTD Upload section via CI. Of course, if you want to steam ahead and provide patches I'm not going to stop you, but I'd rather you keep focus on the ISR/task cleanup for now.

@marcelstoer 👍 to your points above

@TerryE
Copy link
Collaborator

TerryE commented Jan 17, 2016

First, this isn't a philosophical point. It's a pragmatic one. We shouldn't assume that every developer has 24×7 Internet access.
Second this shouldn't delay anything. As I've already said a few times, as you have just done, the new documentation is a material step forward. This is just a tidy up flag for a future todo and a low priority one at that.

@devsaurus
Copy link
Member

transfer the last outstanding module (1-wire)

It's RTD'ed now.

marcelstoer added a commit to marcelstoer/nodemcu-firmware that referenced this issue Jan 17, 2016
@marcelstoer
Copy link
Member Author

All done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants