Code <-> Documentation workflow

luisherranz · 24 April 2020 18:39

Yes, but I was thinking reserving the dev branch for the devrel-team because they may also want to merge to a branch instead of directly to master at some point

mmczaplinski · 24 April 2020 18:49

gotcha, makes sense

David · 29 April 2020 08:08

Let’s try that and see if it works!

I’m going to create the code branch in the gitbook-docs repository. Every PR updating the code documentation should be merged there.

juanma · 25 May 2020 10:55

@dev-team Workflow to add documentation on code releases, documented here:

If there’s any place where do you think this should be referenced please let me know

luisherranz · 28 May 2020 06:50

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:

Check if TypeDoc is in sync: An action that runs all the typedoc scripts and fails if there are uncommitted files in git.
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 · 12 June 2020 08:01

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 · 12 June 2020 08:05

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 · 12 June 2020 08:09

@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 · 12 June 2020 08:12

Oh, I missed it. My bad, sorry

juanma · 12 June 2020 10:03

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 · 12 June 2020 13:15

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:

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 · 24 June 2020 08:13

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 · 24 June 2020 08:14

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

luisherranz · 26 June 2020 07:33

@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 · 26 June 2020 07:36

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 · 26 June 2020 09:22

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 · 26 June 2020 11:04

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)

Captura de pantalla 2020-06-26 a las 13.00.31

luisherranz · 29 June 2020 16:05

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

juanma · 30 June 2020 08:10

I have updated the workflow description in the WIKI:

luisherranz · 30 June 2020 11:14

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.

@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?