medic / angular10-migration

0 stars 0 forks source link

Integrate angular code documentation #120

Open latin-panda opened 3 years ago

latin-panda commented 3 years ago

In AngularJS (1.x) we were using angular-jsdoc but it's not compatible with the newest Angular(+2).

I found 2 possible replacements:

1.. Typedoc which provides class descriptions (properties, methods, contructor), it has limited features. I tried this one out and it doesn't recognise our shared libraries and complains about our loose typing. I don't recommend this one yet.

Expected 1 arguments, but got 0. Did you forget to include 'void' in your type argument to 'Promise'?
514     const nextTick = () => new Promise(resolve => realSetTimeout(() => resolve()));

Cannot find module '@medic/phone-number' or its corresponding type declarations.
5 import * as phoneNumber from '@medic/phone-number'

2.. Compodoc provides: class definitions, documentation coverage analysis, source code view, DOM tree with quick nav, dependencies graph and list, search, custom branding (favicon, logo, hide generated by), serve docs, etc. I quickly tried this one too and noticed 2 issues so far:

So, it's not 100% working fine and we need to figure out these issues or open issues in the Compodoc project. This is the minimum to run Compodoc: npx compodoc -p tsconfig.base.json --disableRoutesGraph -s

See video of Compodoc here

latin-panda commented 3 years ago

I believe this feature is used very little by devs, where usually they read the code right away and use the IDE features to answer their questions. I wonder if there are other parties interested in generating the documentation instead of checking the code directly.

So, let's do a poll: 👍 - if you want Compodoc to generate webapp's documentation. 👎 - if you are okay of removing the Generator of Documentation feature for now. Then we remove the command line option in grunt.

Have another option? Post a comment ✨ Thanks!

@garethbowen @MaxDiz @abbyad @craig-landry @brad1905 @newtewt @ngaruko @mrjones-plip @dianabarsan @njogz @mrsarm

mrsarm commented 3 years ago

My vote is :+1: but,

I don't think we need to generate the webapp's documentation, except maybe for some core components where compodoc may help to understand better the usage of a them with examples. In my case I prefer to read documentation directly from the code, but I do think that write documentation within the code needs a standardized way, so Compodoc may be a good option.

Moreover, if the IDE supports the documentation tool used, it helps you to not forget to document some arguments in a method, or when you rename one... also when you import a method it allows you to see the doc in a tooltip without the need to go to the actual code... all is possible only if we document the code in one way.

Going even further, we should enforce in PRs to always write a doc in any new public method exported in a module or a class, unless it is one line method where the name of the method or the code on it makes too obvious the purpose of the method, eg. function getCurrentMonthName() { return ['Jan', 'Feb', 'Mar', ...][new Date().getMonth()] }

mrjones-plip commented 3 years ago

I gave it at :+1: - but it's a soft :+1: . I think that it's more welcoming to new/external devs and easier for when reading outside an IDE, like in a PR. However, I don't think it's a huge issue.

newtewt commented 3 years ago

We do have the API portion documented but that seems to be done manually. I have no real preference as I'm one to just dive in as well but I think it is a good thing to have for external contributors.

latin-panda commented 3 years ago

Thanks guys for the input!!

Since the API portion is available for devs/app builders to consume and write integrations/tools externally and internally, it makes sense to maintain the documentation. However, Webapp isn't build for being consumed in the same way as API, devs/app builders won't consume functions or outputs outside this app.

Because must of us jump into the code directly to get understanding and because running a command isn't the most friendly/straight forward way for getting documentation (specially for external contributors) I'm going to remove this task from the current iteration of this migration.

Many showed their care for the external contributors, so my suggestion is to improve or create a document either in CHT and/or Forum explaining the architecture and high level tech in Webapp (important libraries, enketo, reduxt approach, etc) as an introduction before the contributors jump into the code, that way they know what to expect. But not giving too much detail (function, variables, classes) that later will be hard to maintain and promp to become obsolete fast -- guessing this is the reason of generating docs in the first place. We'll continue adding comments in code when the code isn't self explanatory.

@abbyad @craig-landry @michaelkohn @garethbowen Does this sounds reasonable?

craig-landry commented 3 years ago

Thanks guys for the input!!

Thank you for your attention and assessment on this @latin-panda. I agree with the importance of fine-grained documentation on the API portion and omitting it from the webapp, where that effort is better spent making the tech/architecture more approachable.

abbyad commented 3 years ago

Pulling together the necessary high level tech/arch info in the doc site seems like a good approach to me. I imagine this would feature heavily in the Core Framework section of the docs site.

mrjones-plip commented 3 years ago

@abbyad's comment reminded me of a recent hacker news post about the importance of these architecture intro docs.

latin-panda commented 3 years ago

Thank you guys for the comments!

I've removed the ticket from this iteration and created a small PR to remove the jsdocs command for webapp.

I'm starting a draft for the high level tech/arch info. I'll reach out to teammates for collaboration and/or feedback, to later send a PR before publishing at CHT Toolkit.