Skip to content

Conversation

@lorensr
Copy link
Contributor

@lorensr lorensr commented Nov 18, 2022

I like these, for helping people know what content it has, and easily jumping to a specific thing. While there is a built-in button, many don't know of it, and it's harder to get a sense of everything because it's small and scrolling.

Updatable either manually or with npx doctoc README.md

@cretz
Copy link
Member

cretz commented Nov 18, 2022

Hrmm, I like it. My only concern is keeping it up to date. I can't find a --check-only type of option for doctoc. Think instead we can run it in CI and confirm using a git command that README.md didn't change?

I do this with proto gen today. And I have already marked one matrix run as docsTarget. So, including whatever npm setup is needed, you could add:

      # Confirm README ToC is generated properly
      - name: Check generated README ToC
        if: ${{ matrix.docsTarget }}
        run: |
          npx doctoc README.md
          [[ -z $(git status --porcelain README.md) ]] || (git diff README.md; echo "README changed"; exit 1)

(ideally node install would either be same step or at least right near it with same conditional...feel free to push a known failure to confirm the check works before pushing the proper)

@lorensr
Copy link
Contributor Author

lorensr commented Nov 20, 2022

Confirming that the new step fails as expected:

build-lint-test (3.11, ubuntu-latest)

Now fixing so it'll pass

Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Will merge once CI completes.

if: ${{ github.ref == 'refs/heads/main' && matrix.docsTarget }}
run: npx vercel deploy build/apidocs -t ${{ secrets.VERCEL_TOKEN }} --name python --scope temporal --prod --yes

# Confirm README ToC is generated properly
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, thanks!

@cretz cretz merged commit e37b500 into main Nov 22, 2022
@cretz cretz deleted the toc branch November 22, 2022 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants