Skip to content

Conversation

alexandreferris
Copy link
Member

@alexandreferris alexandreferris commented Sep 10, 2025

  • Add docs-verification.yml

PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

There was no way of verifying if wire-docs would continue to build properly from the changes in our docs/ folder

Solutions

Add workflow to verify that everything is still running correctly and won't break on wire-docs side.

Summary by CodeRabbit

  • Documentation

    • Corrected a symlink path in the docs to ensure referenced pages resolve correctly.
    • Updated the “App Permissions” page title to “Permissions” for clarity.
  • Chores

    • Introduced an automated documentation verification workflow that runs on pull requests affecting docs and on manual trigger, ensuring the docs build successfully against the proposed changes.

@alexandreferris alexandreferris requested a review from a team as a code owner September 10, 2025 15:23
@alexandreferris alexandreferris self-assigned this Sep 10, 2025
Copy link

coderabbitai bot commented Sep 10, 2025

Walkthrough

Adds a Docs verification GitHub Actions workflow that builds wire-docs with the current PR branch as the submodule source. Also adjusts a docs symlink path in README and shortens a page title in permissions.md.

Changes

Cohort / File(s) Summary
Docs verification workflow
.github/workflows/docs-verification.yml
New workflow runs on workflow_dispatch and PRs touching docs/**. Checks out repo, sets up Java 17, installs Nix, clones wire-docs, rewrites wire-apps-jvm-sdk submodule URL/branch to current PR context, prints updated .gitmodules, and runs make build.
Docs updates
docs/README.md, docs/src/permissions.md
README: adjusts symlink path from ../../../... to ../../... in “Moving Pages”. permissions.md: changes H1 from “App Permissions” to “Permissions”.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Dev as Developer
    participant GH as GitHub Actions
    participant Runner as Ubuntu 24.04 Runner
    participant Repo as Current Repo
    participant Docs as wire-docs

    Dev->>GH: Open PR (changes under docs/**) or workflow_dispatch
    GH->>Runner: Start job docs-verification
    rect rgb(235, 245, 255)
      note right of Runner: Setup
      Runner->>Repo: actions/checkout
      Runner->>Runner: Setup Java 17 (Zulu)
      Runner->>Runner: Install Nix (cachix/install-nix-action)
    end
    rect rgb(240, 255, 240)
      note right of Runner: Prepare docs sources
      Runner->>Docs: git clone wire-docs
      Runner->>Runner: Derive CURRENT_REPO_URL and CURRENT_BRANCH
      Runner->>Docs: Update .gitmodules (URL & branch for wire-apps-jvm-sdk)
      Runner->>Runner: Show updated .gitmodules
    end
    rect rgb(255, 250, 235)
      note right of Runner: Build
      Runner->>Docs: make build
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • bbaarriiss

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title concisely captures the main change—adding a docs verification workflow—follows semantic commit conventions with the JIRA reference, and avoids extraneous details or vague terms.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Poem

In burrows of docs, I hop with glee,
A workflow blooms for all to see.
Symlinks nudge a step or two,
Titles trim to something new.
I thump “build!”—the pages sing,
Carrots raised for everything. 🥕✨

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/add_workflow_for_docs

Comment @coderabbitai help to get the list of available commands and usage tips.

* Rename docs verification for workflow
Copy link

@mohitrajain mohitrajain left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (5)
docs/src/permissions.md (1)

1-1: Confirm inbound links/anchors after title change.

Changing H1 from “App Permissions” to “Permissions” may break deep links or nav anchors in wire-docs. Please verify nav, xrefs, and any inbound links.

.github/workflows/docs-verification.yml (4)

1-2: Lock down default token permissions.

Declare least-privilege to avoid unexpected writes.

 name: Docs
+permissions:
+  contents: read

9-11: Avoid duplicate runs per-PR head.

Add a concurrency group to auto-cancel superseded builds.

 on:
   workflow_dispatch:
   pull_request:
     paths:
       - 'docs/**'
+
+concurrency:
+  group: docs-verification-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true

11-12: Set an upper bound on job runtime.

Prevents hung builds from consuming runners indefinitely.

   docs-verification:
-    runs-on: ubuntu-24.04
+    runs-on: ubuntu-24.04
+    timeout-minutes: 30

26-37: Trim trailing spaces flagged by YAMLlint.

Clean up lines 26, 28, 34, 37 to satisfy linters.

-          
+ 

(Apply similarly to the other noted lines.)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21c988b and 3400027.

📒 Files selected for processing (3)
  • .github/workflows/docs-verification.yml (1 hunks)
  • docs/README.md (1 hunks)
  • docs/src/permissions.md (1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/docs-verification.yml

[error] 26-26: trailing spaces

(trailing-spaces)


[error] 28-28: trailing spaces

(trailing-spaces)


[error] 34-34: trailing spaces

(trailing-spaces)


[error] 37-37: trailing spaces

(trailing-spaces)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: tests
🔇 Additional comments (1)
docs/README.md (1)

51-52: Validate the new symlink from the correct directory
The README.md snippet lives under docs/, not src/integrations. From the repo root run:

cd docs
test -e ../wire-apps-jvm-sdk/docs/src/creating-an-app.md

Likely an incorrect or invalid review comment.

Comment on lines +22 to +39
- name: Clone wire-docs and build
run: |
git clone --branch main https://github.com/wireapp/wire-docs.git wire-docs
cd wire-docs

CURRENT_REPO_URL="https://github.com/${GITHUB_REPOSITORY}.git"

if [ -n "$GITHUB_HEAD_REF" ]; then
CURRENT_BRANCH="$GITHUB_HEAD_REF"
else
CURRENT_BRANCH="${GITHUB_REF#refs/heads/}"
fi

sed -i "s|url = https://github.com/wireapp/wire-apps-jvm-sdk.git|url = ${CURRENT_REPO_URL}|g" .gitmodules
sed -i "s|branch = main|branch = ${CURRENT_BRANCH}|g" .gitmodules

cat .gitmodules
make build
Copy link

@coderabbitai coderabbitai bot Sep 10, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Submodule isn’t initialized/updated after editing .gitmodules.

Without sync/update, make build may use a stale/missing submodule.

Append after the .gitmodules edits:

           sed -i "s|url = https://github.com/wireapp/wire-apps-jvm-sdk.git|url = ${CURRENT_REPO_URL}|g" .gitmodules
           sed -i "s|branch = main|branch = ${CURRENT_BRANCH}|g" .gitmodules
           
           cat .gitmodules
+          git submodule sync --recursive
+          git submodule update --init --recursive --checkout --remote
           make build
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Clone wire-docs and build
run: |
git clone --branch main https://github.com/wireapp/wire-docs.git wire-docs
cd wire-docs
CURRENT_REPO_URL="https://github.com/${GITHUB_REPOSITORY}.git"
if [ -n "$GITHUB_HEAD_REF" ]; then
CURRENT_BRANCH="$GITHUB_HEAD_REF"
else
CURRENT_BRANCH="${GITHUB_REF#refs/heads/}"
fi
sed -i "s|url = https://github.com/wireapp/wire-apps-jvm-sdk.git|url = ${CURRENT_REPO_URL}|g" .gitmodules
sed -i "s|branch = main|branch = ${CURRENT_BRANCH}|g" .gitmodules
cat .gitmodules
make build
- name: Clone wire-docs and build
run: |
git clone --branch main https://github.com/wireapp/wire-docs.git wire-docs
cd wire-docs
CURRENT_REPO_URL="https://github.com/${GITHUB_REPOSITORY}.git"
if [ -n "$GITHUB_HEAD_REF" ]; then
CURRENT_BRANCH="$GITHUB_HEAD_REF"
else
CURRENT_BRANCH="${GITHUB_REF#refs/heads/}"
fi
sed -i "s|url = https://github.com/wireapp/wire-apps-jvm-sdk.git|url = ${CURRENT_REPO_URL}|g" .gitmodules
sed -i "s|branch = main|branch = ${CURRENT_BRANCH}|g" .gitmodules
cat .gitmodules
git submodule sync --recursive
git submodule update --init --recursive --checkout --remote
make build
🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 26-26: trailing spaces

(trailing-spaces)


[error] 28-28: trailing spaces

(trailing-spaces)


[error] 34-34: trailing spaces

(trailing-spaces)


[error] 37-37: trailing spaces

(trailing-spaces)

🤖 Prompt for AI Agents
In .github/workflows/docs-verification.yml around lines 22 to 39, after the two
sed edits to .gitmodules and before running make build, ensure the repo
submodules are synced and initialized/updated; append commands to run a
recursive git submodule sync and then git submodule update --init --recursive
(optionally with --remote) so the modified .gitmodules takes effect and the
build uses the updated submodule content.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@alexandreferris Isn't CodeRabbit actually right here?

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Choose a reason for hiding this comment

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

Copy link
Collaborator

@spoonman01 spoonman01 left a comment

Choose a reason for hiding this comment

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

It looks a bit extra complicated to me, but as far as it only tries to check during the PR that the docs are fine, without deploying anything or similar, then it's okay

@mohitrajain
Copy link

@spoonman01 It is just trying to build the docs in the git runner environment, it doesn't push any changes anywhere. We need this logic to ensure changes in /docs in your repository won't impact the building of whole wire-docs project as it is shared by other teams/repos as well. We have a similar flow for wire-server:
https://github.com/wireapp/wire-server/blob/develop/.github/workflows/build.yaml

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.

3 participants