Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(chore) Only do certain checks for documentation changes (github workflow) #3898

Open
weedySeaDragon opened this issue Dec 8, 2022 · 5 comments
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect

Comments

@weedySeaDragon
Copy link
Contributor

weedySeaDragon commented Dec 8, 2022

Description

There are specific github workflows and checks unique to documentation (but not code):

  • checking links

Likewise, there are workflows and checks specific to code and not documentation:

  • unit tests
  • end to end tests (cypress e2e)
  • test static analysis

We should modify the github workflow(s) so that only the workflows & checks required for the context (documentation or code) are run. If a PR includes changes to both documentation and code, then all relevant workflows/checks should be run.

Recently the link checker has been failing because of some issue communicating with a documentation link destination. This is happening even if only code was changed. It means that a PR fails and a contributor must be involved and review and resolve/override/re-run the failing check. Obviously this means it takes more time for the PR to be completed and means that more people have to be invovled.

Only running the necessary workflows/checks will reduce PR submission friction for contributors: it makes the process faster and smoother, which makes it more likely that more contributions will be made because it's less onerous.

@sidharthv96 - thoughts?

Here's what the flow might look like:
Mermaid Live version

flowchart TD
    
    PR_submit(["PR opened"]) ==> changedFiles-Docs[[changed-files action:\ncheck src/docs/*]]
    changedFiles-Docs --> DocsChanged{Any src/docs\n files changed?}
    DocsChanged -->|yes| DocsJobs
    subgraph DocsJobs["Documentation Jobs"]
        direction LR
        linkChecker["lycheeverse/lychee-action\nLink checker"]
        readmeSync[check-readme-in-sync]
        subgraph LintDocs["Lint Docs"]
            verifyDocs[[pnpm run docs:verify]] --> DocVerifyResult{docs:verify\n failed?}
            DocVerifyResult -->|yes| buildDocs[[pnpm run docs:build]]
            buildDocs --> commitDocs[[Commit docs changed]]

        end
    end
    PR_submit ==> changedFiles-Other
    changedFiles-Other[[changed-files action:\ncheck !src/docs/*]]
    changedFiles-Other --> OtherFilesChanged{"Any !src/docs\n (not docs)\n files changed?"}
    OtherFilesChanged -->|yes| BuildJobs
    subgraph BuildJobs["Build (other files) Jobs"]
        direction LR
        build[[build]] --> uploadBuild[["Upload Mermaid Build as Artifact"]]
        uploadBuild --> uploadMindmadBuild[["Upload Mindmap build as Artifact"]]
        unitTests[Unit tests]
        cypressSpecCheck["checks.yml\n(check cypress files)"]
        codeQl["github/codeql-action/analyze\nPerform CodeQL analysis"]
        dependencyReview["dependency-review-action"]
        e2e[e2e tests]
        unitTests[[pnpm run ci --coverage]] --> coverageUpload[["coverallsapp/github-action@master"]]
    end
Loading

Note that the shapes are inconsistent; I was playing around with them.

Steps to reproduce

Only change a wee bit of documentation.
Submit a new PR.
Note all of the code-specific checks/workflows/actions that have to be run.

Screenshots

No response

Code Sample

No response

Setup

No response

Additional Context

No response

@weedySeaDragon weedySeaDragon added Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Dec 8, 2022
@weedySeaDragon
Copy link
Contributor Author

Here's a workflow action that could be used: Changed Files

@sidharthv96
Copy link
Member

Running link checker only on documentation changes is an excellent idea.
This might introduce a small issue, though. Now link checker is running in all PRs, and it might be keeping the cache warm.
If we switch it to only documentation PRs, it's almost guaranteed to fail, with GitHub links throwing 429s on the first run.

We can experiment with the cache duration and maybe run the workflow using cron.

@sidharthv96
Copy link
Member

Can we use the native feature https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-including-paths ?

I had experimented with changed-files for the auto build docs pr here 9c5c85d. It works pretty well, but is an extra dependency.

@weedySeaDragon
Copy link
Contributor Author

Sure -- the native feature would work.
What are the limits of how many dependencies are "too many"? It's a classic "build vs. buy" trade-off: someone else is responsible for maintaining that code vs. more code to maintain in this project.
Just trying to get a handle on what the priorities / loads/ costs are for this project.

Not a big deal to me - I think the bigger payoff will be making documentation easier and faster to submit, and likewise code submission won't get hung-up on documentation-only problems.
(Of course I hope that every time someone adds a new feature they also update the documentation, in which case there's no time savings.)

@sidharthv96
Copy link
Member

Just trying to get a handle on what the priorities / loads/ costs are for this project.

I don't think we have that defined anywhere (a good contender in contributin.md maybe?).
But my instinct is to use native functions if it's available and good enough for the job without adding too much complexity, and fallback to external deps if they're easier to integrate.

#3434 is a good example. We were already using lodash as dependency, and the handwritten memoize function was causing us some trouble.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

No branches or pull requests

2 participants