ci(i): Add check to keep table of contents up-to-date #2693

shahzadlone · 2024-06-07T13:13:19Z

Relevant issue(s)

Resolves #2692

Description

Combine previous documentation workflows in one file
Add toc script tool
Add the toc make rule
Add toc job

How has this been tested?

Using act tool
Manually in the commit which was reverted to ensure it fails if Readme is not updated.
Action-run: https://github.com/sourcenetwork/defradb/actions/runs/9417618174/job/25943201802?pr=2693

Specify the platform(s) on which this was tested:

WSL2

shahzadlone · 2024-06-07T13:25:46Z

@fredcarle no more code 12 + enter hahaha

AndrewSisley · 2024-06-07T13:28:26Z

Combine previous documentation workflows in one file

question: What made you chose to do this?

codecov · 2024-06-07T13:29:44Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.98%. Comparing base (0c134e5) to head (a4cd76a).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2693      +/-   ##
===========================================
+ Coverage    77.91%   77.98%   +0.08%     
===========================================
  Files          308      308              
  Lines        23134    23134              
===========================================
+ Hits         18023    18041      +18     
+ Misses        3723     3711      -12     
+ Partials      1388     1382       -6

Flag	Coverage Δ
all-tests	`77.98% <ø> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 6 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c134e5...a4cd76a. Read the comment docs.

AndrewSisley · 2024-06-07T13:30:52Z

tools/scripts/md-toc/gh-md-toc

@@ -0,0 +1,421 @@
+#!/usr/bin/env bash
+


thought: 😅 Please never ask me to maintain this file. Are we really sure this is worth it?

You/we don't have to. I been using this for a few years now and has 3.2k stars. Is a keep it and forget it kind of thing IMO.

Worst case scenario, if something does break, we copy the new version with the fix and replace this script.

It is copy-pasted as-is? No changes from the source? Why have we copy pasted it?

It is copy-pasted as-is? No changes from the source?

yes. yes no changes.

Why have we copy pasted it?

Is a simple bash script. Is going to be painful to have devs manually download and making sure they have same version etc (specially if there are formatting changes between versions).

The readme pasted in suggests that we wouldn't need devs to manually download it (unless maybe it requires sudo?)

It also looks like the script doesn't work on windows:

If you want it on Windows, you better to use a golang based implementation

todo: Why don't we just use the go version instead of the mac/linux-only bash script, then we wont need to copy-paste code about?

go install "github.com/ekalinin/github-markdown-toc.go/cmd/gh-md-toc@latest" etc...

I haven't used the go version of the tool personally before, I wonder if formatting and everything else is the same.

I tried switching to the go project, but sadly the go project is missing the insert option, and only generates the table of contents (i.e. doesn't update it automatically). So will prefer the current approach.

ekalinin/github-markdown-toc.go#26

@sourcenetwork/database-team Please chime in if you have any other suggestions regarding the copy-paste and lack of window dev tool support (or another tool). Happy to follow the consensus.

If we never have to maintain this, I don't really care that we just copy-pasted it. I also don't think it's a big deal that it doesn't work on windows. Devs on windows can just manually edit if they have to. If someone what's to contribute to the go version to include insert, we can switch to the go version when it's supported.

If we never have to maintain this, I don't really care that we just copy-pasted it. I also don't think it's a big deal that it doesn't work on windows. Devs on windows can just manually edit if they have to. If someone what's to contribute to the go version to include insert, we can switch to the go version when it's supported.

I am okay with this! I have subscribed to the issue, if it gets implemented happy to migrate to using go version of the tool.

shahzadlone · 2024-06-07T13:53:52Z

Combine previous documentation workflows in one file

question: What made you chose to do this?

No hard pref or reason just some minor ones, seems nicer to have all documentation jobs in one workflow file (they still behave in parallel manner and can be controlled granularly to make required or not as that is based on the job name.)

One nitpicky thing I like is that the github check mark will show: Check Documentation Workflow / Check http documentation rather than Check Http Documentation Workflow / Check http documentation

Everything remains the same, except we loose the ability to configure the trigger (push and pull) in a granular manner per workfllow vs same for all jobs now, but we will always have same for all these jobs anyways so doesn't matter.

If there is a hard pref to keep them in separate workflow files lmk, as mentioned I am in different.

AndrewSisley · 2024-06-07T13:57:23Z

if there is a hard pref to keep them in separate workflow files lmk, as mentioned I am in different.

No real preference from me at all atm :) I was just curious :) Thanks for the explanation

fredcarle

LGTM. Please fix the table of content header syntax before merging.

README.md

fredcarle · 2024-06-07T18:29:49Z

tools/scripts/md-toc/gh-md-toc

@@ -0,0 +1,421 @@
+#!/usr/bin/env bash
+


If we never have to maintain this, I don't really care that we just copy-pasted it. I also don't think it's a big deal that it doesn't work on windows. Devs on windows can just manually edit if they have to. If someone what's to contribute to the go version to include insert, we can switch to the go version when it's supported.

This reverts commit d9f2b90.

shahzadlone added documentation Improvements or additions to documentation ci/build This is issue is about the build or CI system, and the administration of it. labels Jun 7, 2024

shahzadlone added this to the DefraDB v0.12 milestone Jun 7, 2024

shahzadlone self-assigned this Jun 7, 2024

shahzadlone changed the title ~~ci(i): Add check to ensure readme table of content is in sync~~ ci(i): Add check to keep table of content up-to-date Jun 7, 2024

shahzadlone requested a review from a team June 7, 2024 13:19

shahzadlone force-pushed the lone/ci/readme-md-toc branch from ce546e3 to c8c7da6 Compare June 7, 2024 13:24

AndrewSisley reviewed Jun 7, 2024

View reviewed changes

shahzadlone changed the title ~~ci(i): Add check to keep table of content up-to-date~~ ci(i): Add check to keep table of contents up-to-date Jun 7, 2024

fredcarle approved these changes Jun 7, 2024

View reviewed changes

shahzadlone force-pushed the lone/ci/readme-md-toc branch from c8c7da6 to 43c08ef Compare June 7, 2024 19:28

shahzadlone added 7 commits June 7, 2024 15:30

PR: Add the markdown toc bash tool + its README

cc07418

PR: Add the make toc rule to fix README's TOC

e5df4eb

PR: Add the placeholders & do make toc command

e65b5ea

PR: Combine all documentation workflows in a file

fc7dde0

PR: Add job to detect readme toc out of date

ef37ce3

PR(REVERT): undo this commit after testing action

b2bae05

Revert "PR(REVERT): undo this commit after testing action"

a4cd76a

This reverts commit d9f2b90.

shahzadlone force-pushed the lone/ci/readme-md-toc branch from 43c08ef to a4cd76a Compare June 7, 2024 19:30

shahzadlone merged commit 343b90a into sourcenetwork:develop Jun 7, 2024
37 of 38 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci(i): Add check to keep table of contents up-to-date #2693

ci(i): Add check to keep table of contents up-to-date #2693

shahzadlone commented Jun 7, 2024 •

edited

Loading

shahzadlone commented Jun 7, 2024

AndrewSisley commented Jun 7, 2024

codecov bot commented Jun 7, 2024 •

edited

Loading

AndrewSisley Jun 7, 2024

shahzadlone Jun 7, 2024

AndrewSisley Jun 7, 2024

shahzadlone Jun 7, 2024 •

edited

Loading

AndrewSisley Jun 7, 2024 •

edited

Loading

shahzadlone Jun 7, 2024

shahzadlone Jun 7, 2024 •

edited

Loading

shahzadlone Jun 7, 2024

fredcarle Jun 7, 2024

shahzadlone Jun 7, 2024

shahzadlone commented Jun 7, 2024

AndrewSisley commented Jun 7, 2024

fredcarle left a comment

fredcarle Jun 7, 2024

ci(i): Add check to keep table of contents up-to-date #2693

ci(i): Add check to keep table of contents up-to-date #2693

Conversation

shahzadlone commented Jun 7, 2024 • edited Loading

Relevant issue(s)

Description

How has this been tested?

shahzadlone commented Jun 7, 2024

AndrewSisley commented Jun 7, 2024

codecov bot commented Jun 7, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shahzadlone Jun 7, 2024 • edited Loading

Choose a reason for hiding this comment

AndrewSisley Jun 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shahzadlone Jun 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shahzadlone commented Jun 7, 2024

AndrewSisley commented Jun 7, 2024

fredcarle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shahzadlone commented Jun 7, 2024 •

edited

Loading

codecov bot commented Jun 7, 2024 •

edited

Loading

shahzadlone Jun 7, 2024 •

edited

Loading

AndrewSisley Jun 7, 2024 •

edited

Loading

shahzadlone Jun 7, 2024 •

edited

Loading