Code <-> Documentation workflow

[luisherranz]

I have an idea to keep the markdown generated by TypeDoc in sync with the codebase changes.

Previous steps:

• I am going to do a PR to add the typedoc script to all our packages. As suggested by @juanma, I am going to export the markdown in a tsdoc folder inside each package.

Once that is done, I am going to create two GitHub actions:

1. Check if TypeDoc is in sync: An action that runs all the typedoc scripts and fails if there are uncommitted files in git.
2. Run the typedoc scripts: An action that can be triggered by a comment (with the technique discovered by @mmczaplinski https://github.com/nwtgck/actions-comment-run) and runs the typedoc scripts and creates a new commit with the changes.

The main goal of using this vs a more automatic approach that generates new commits on each push is to try to keep the commit history clean of too many “Update typedocs” commits.

Thanks to the first action, we can make sure that we won’t merge a PR out of sync. Thanks to the second action, we don’t need to use the local repo to sync it.

Actually, it’s the very same workflow than the “Out of sync with dev branch” that GitHub has, that doesn’t let you merge until you’ve merged the latest dev changes, and there’s a button for doing that manually in the GitHub UI.

The normal workflow would be:

• Create PR…
• Work on your code…
• Once it’s ready for review, run npm run typedoc locally to generate the markdown.
• (if you forget to run the command locally you can alternatively use the GH comment that runs the GH action that runs the script)
• Push everything. The GH action that checks that markdown is in sync should pass.
• Code reviewer can review the code.
• Doc reviewer can see the markdown changes of this PR and open the Doc PR.
• If changes are requested, owner will work on them and push the new code.
• If those changes made the markdown to be out of sync, the GH action will fail and won’t allow the merge.
• Once the changes are finished, the owner re-requests a review.
• Thanks to the GH action, owner and reviewers can see if the markdown is in sync or not. And thanks to the GH comment, they can just sync it again without needing to run the repo locally or ask the owner to do so.

Let me know what you think!

[luisherranz]

I’ve just realized we have two different threads with overlapping conversations. This one and Adopting a standard way to document Frontity APIs. I’m going to close that one in favor of this one.

---

To keep this one up to date: Our typedoc approach didn’t work as expected. More info about the reasons here: Adopting a standard way to document Frontity APIs. We are going to write our APIs manually for now.

@documentation-team, maybe you can come up with a markdown template that we can use across all the documentation to standardize the APIs? Something that is easy to use to move from TSDocs to markdown, even if it has to be manually copied and pasted. What do you think?

[luisherranz]

Also, to keep this thread up to date, Juanma has just updated the Code <-> Docs guideline: https://github.com/frontity/gitbook-docs/wiki/Code-Releases

@Juanma, I think the only thing missing is that the Doc PR should be reviewed by the owner of the code PR. Does it make sense?

[juanma]

@luisherranz

@Juanma, I think the only thing missing is that the Doc PR should be reviewed by the owner of the code PR. Does it make sense?

@luisherranz isn’t this explained here? → https://github.com/frontity/gitbook-docs/wiki/Code-Releases

• The developer in charge of the code PR will be added as reviewer of the docs PR
• The approval of the developer in charge of the code PR will be required to merge the docs PR

[luisherranz]

Oh, I missed it. My bad, sorry

[juanma]

I’ve done a quick test of documenting some API methods using a specific format. It’s just like a template we can use to document API’s in a standard way

https://docs.frontity.org/api-reference-1/wordpress-source#source-fetch

{% hint style="info" %}

`{ (link: string, options: object) => Promise }`

- **Parameters**
  - `link { string }` Link representing a REST API endpoint or custom handler 
  - `options { object }`
    - `force { boolean }`: The entities should be fetched again.

- **Return value**
  - `{ Promise }` Promise resolving to data fetched

{% endhint %}

Another example → https://docs.frontity.org/api-reference-1/wordpress-source#source-get

[luisherranz]

I think it’s great @juanma

The only thing I think it’s a bit confusing is the use of brackets. I’d use a more “typescripish” syntax, using colons:

{% hint style="info" %}

`(link: string, options: object) => Promise`

- **Parameters**
  - `link: string` Link representing a REST API endpoint or custom handler 
  - `options: object`
    - `force: boolean`: The entities should be fetched again.

- **Return value**
  - `Promise` Promise resolving to data fetched

{% endhint %}

Other than that I think it’s great

By the way, just so you can have a better picture I have done a PR with the proper TSDoc for that part of the code:

Add TSDocs for actions.source.fetch

interface WpSource extends Source {
  // ...
  actions: {
    source: {
      /**
       * An action that fetches all the entities related to a link, and
       * populates the state with both an entry in `state.source.data` with
       * information about that link and the entities, normalized, in the
       * relevant part of the state (like `state.source.post`,
       * `state.source.category`, `state.source.author` and so on).
       *
       * @param link - The link that should be fetched. It can be a URL or a
       * custom link created to fetch additional entities from the REST API.
       * - URLs start with "/".
       * - Non URLs start with "@".
       *
       * @example Fetch the content of the `/some-post` URL:
       * `actions.source.fecth("/some-post");`
       *
       * @example Fetch the comments of the post with id 135:
       * `actions.source.fecth("@comments/135");`
       *
       * @param options - Optional options.
       *
       * @returns A promise that resolves when the data fetching has finished.
       */
      fetch:
        | AsyncAction<WpSource, string>
        | AsyncAction<
            WpSource,
            string,
            {
              /**
               * Whether the fetch should be done again if data for that link
               * already exists.
               */
              force?: boolean,
            }
          >,

      /**
       * An internal action that bootstraps the initialization
       *
       * This action is not meant to be run by the user, but by the Frontity
       * framework.
       */
      init: Action<WpSource>,
    },
  };
}

Let me know if this helps or if you think we could do something in a different way.

[juanma]

The ideal scenario could be when the code PR is merged:

If the docs PR is already approved

It automatically merges it (docs PR)

If the docs PR is still opened

The proper message will be set in the docs PR. So when the docs PR is closed we can manually merge it if the code PR is already merged

[juanma]

Maybe we can use a tool like this one to automate the merge of the docs PR when the code PR is merged

probot/probot

[luisherranz]

@documentation-team we now have two different cases of documentation tasks that require more than simple “API” updates:

• The Slot and Fill pattern.
• The Analytics packages. Think about how do you want to approach this and let us know

It doesn’t need to be the final process, we can start trying things out until we find one that suits both teams, of course.

[juanma]

A good first step would be creating an issue here for each one of these topics → https://github.com/frontity/docs/issues so you or any developer can add there any info you think it should be taken into account
From there we can start a conversation about each topic and create proper docs

Regarding “Slot and Fill” there’s this PR → https://github.com/frontity/docs/pull/90
When @Michael gets all the info I think some related docs about the “The Slot and Fill pattern” could emerge from that PR

[luisherranz]

@juanma

A good first step would be creating an issue

Oh, I like it! Creating an issue would be way simpler than the workflow we have now, even for “API” updates. That way the dev-team wouldn’t depend on you to be able to merge the code PR.

We could have a label to mark the issues that are meant to be created with priority to keep code <-> docs in sync, and you could use that label to prioritize your tasks.

This would mean that instead of the somehow difficult process that we have now where we need to assign you in our PR, wait for you to create a Doc PR and finally mark the task as complete, it would be as simple as completing this task:

- [ ] Open an issue in the Documentation repo (and link it here).

We can use labels and even the description to complete the issue with useful information for the devrel team.

What do you think guys?

cc: @documentation-team @dev-team @SantosGuillamot

[juanma]

Yes, I think it’s much simple in this way

We have this labels already created to assign a priority to these issues.
Maybe you can just directly assign these issues a label → priority: 2 high (along with any other labels you think it may help locate the issue better)

[luisherranz]

I have edited the PR template to reflect this change: https://github.com/frontity/frontity/commit/ca1414f89102d002f2a79f57e4b58759229d98b0

[juanma]

I have updated the workflow description in the WIKI:

frontity/docs

[luisherranz]

Hey guys, I’d love if you could check out this PR and give feedback here about how the TSDocs for the types.ts file of a Frontity package can look like.

Add TSDocs for actions.source.fetch

---

@dev-team, @SantosGuillamot:

• What do you think?
• Do you do something different?

@documentation-team:

• Is it useful for you?
• Do you want us to do something different?

[luisherranz]

Another thing: @documentation-team, what are you using to prettify the markdown files? Isn’t it prettier?

I tried to do a simple change to a markdown file in my local environment and prettier introduced a lot of changes to the format: https://github.com/frontity/docs/commit/16e81a482e23b28aec36485c58195e0df94328b5

[mmczaplinski]

I’ve reviewed the PR that @luisherranz has made:

Add TSDocs for actions.source.fetch

I think it looks great, I have only noticed one thing that I think we should all agree on.

In TSDocs or in Documentation, when describing a method, I think we should be adding the () after the method name. Right now, we write it like (examples from current TSDocs):

An object that will be used in each call to the REST API when using actions.source.fetch with the default handlers.

Usually returned from api.get, but can also be the one returned by fetch.

Especially in the second example, it might not be clear to a new user that get and fetch are both methods and not some kind of getter or flag.

[juanma]

@luisherranz

Another thing: @documentation-team, what are you using to prettify the markdown files? Isn’t it prettier ?

We’re not using prettier for code examples in markdown files. But I think we should.
We should add a prettier configuration file so we all use the same prettier configuration in the docs.
Which one are you using for frontity code?

[luisherranz]

@juanma

Which one are you using for frontity code?

Prettier without any configuration. It’s already in the package.json

You can add it to your pre-commit hook if you want, using lint-staged using prettier --write (https://github.com/okonet/lint-staged#running-multiple-commands-in-a-sequence)

Optionally, you can configure VS Code to run prettier on file save with the “Format on save” option.