-
Notifications
You must be signed in to change notification settings - Fork 0
chore: Add sample documentation #WPB-19997 #185
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
Conversation
* Add apps folder in docs folder * Add README, permissions and create-an-app .md files * Add an image for when creating an app
WalkthroughAdds four new documentation pages: a repository management guide (docs/README.md) detailing submodule-based workflows and build/validation steps, plus three user-facing docs under docs/src covering Apps overview, creating an app, and app permissions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant SDK as wire-apps-jvm-sdk
participant DOCS as wire-docs (MkDocs)
participant CI as CI Build
Dev->>SDK: 1) Edit docs in SDK repo<br/>open PR (branch)
Note right of SDK: Submodule referenced by wire-docs
CI->>DOCS: 2) Build docs switching submodule<br/>to SDK PR branch/commit
DOCS-->>CI: Validation result (links, nav, build)
Dev->>DOCS: 3) Open PR to update submodule<br/>to validated SDK commit
CI->>DOCS: 4) Publish on merge (pin exact commit)
Note over DOCS,CI: Current: manual submodule updates<br/>Planned: automate
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
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.
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
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (3)
docs/apps/permissions.md (1)
13-15
: Validate example mappings to scopes.A “standup bot” typically needs to read user check-ins, so READ/WRITE is more appropriate; WRITE fits notification-only bots. Please confirm the intended behavior and adjust.
-### Example Scenarios -- A **standup bot** might only need `WRITE`. -- A **reporting app** might need `READ/WRITE`. +### Example scenarios +- A **standup bot** typically needs `READ/WRITE`. +- A **notifications app** (e.g., CI alerts) usually needs `WRITE`.docs/apps/creating-an-app.md (2)
32-37
: Clarify WRITE in table.Add “cannot receive messages” to keep consistency with the text and permissions.md.
-| `WRITE` | Write only Apps. | +| `WRITE` | Write‑only Apps; cannot receive messages. |
38-39
: Punctuation and style.End the sentence with a period.
-To learn more about permissions, [click here](./permissions.md) +To learn more about permissions, [click here](./permissions.md).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/apps/images/create-app.png
is excluded by!**/*.png
📒 Files selected for processing (3)
docs/apps/README.md
(1 hunks)docs/apps/creating-an-app.md
(1 hunks)docs/apps/permissions.md
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/apps/permissions.md
5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
7-7: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
9-9: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
docs/apps/README.md
5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
7-7: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
9-9: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ 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
* Apply coderabbit changes
* Apply coderabbit changes
…into chore/wpb_19997add_sample_docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/apps/permissions.md (1)
5-11
: Fix heading level, remove trailing colon, flatten bullets, and tighten wording (resolves MD001/MD026/MD007).Updates heading to H2, removes the colon, converts nested bullets to single-line items, and fixes grammar/capitalization.
-### Permission Types: -- **ALL**: Can do everything a normal User can. - - Reserved for legal hold, archival bots, etc. are permanently online and active. -- **READ/WRITE**: Utility Apps. - - Users interact with these Apps in limited ways, on demand using some command like /poll (in contrast to type 1). Think polls, summary bots, Jira integrations, confluence integrations. -- **WRITE**: Utility Apps. - - Apps that notify you about things happening elsewhere and that need a very limited security scope. GitHub notifications, pager duty, jenkins bot, birthday bot. They do not receive messages. +## Permission types + +- ALL — Can do everything a normal user can. Reserved for legal hold and archival bots that are permanently online and active. +- READ/WRITE — Can read messages and post replies. For utility apps users invoke on demand (for example, /poll): polls, summarizers, Jira/Confluence integrations. +- WRITE — Write-only; cannot read messages or receive events. For notifier apps that post updates from external systems (GitHub, PagerDuty, Jenkins, birthday reminders).
🧹 Nitpick comments (1)
docs/apps/permissions.md (1)
13-15
: Align section level and style with the updated heading.Use H2 and sentence case for consistency.
-### Example Scenarios +## Example scenarios - A **reporting app** might need `READ/WRITE`. - A **standup bot** might only need `WRITE`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/apps/README.md
(1 hunks)docs/apps/creating-an-app.md
(1 hunks)docs/apps/permissions.md
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/apps/README.md
- docs/apps/creating-an-app.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/apps/permissions.md
5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
7-7: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
9-9: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ 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/apps/permissions.md (1)
10-11
: Confirm WRITE semantics (events vs. messages).If WRITE apps still receive webhook events but not messages, adjust text; if they receive neither, current wording is fine. Please verify against the SDK behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but one more pair of eyes could be nice to have
IMHO this is okay, we will update soon in any case after integrating backend APIs, we just need the integration with docs.wire done now. I agree however that the closer this docs are to the code, the more chances that they get kept updated, so for me it's okay |
* Add default README on how to work with documentation * Rename apps folder to src
* Rename wire-server references to wire-apps-jvm-sdk * Adjust diagrams to include SDK doc files
There was a problem hiding this 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 (11)
docs/src/permissions.md (2)
13-15
: Tighten examples and formatting.Minor style improvements and consistency with preceding section.
-### Example Scenarios -- A **reporting app** might need `READ/WRITE`. -- A **standup bot** might only need `WRITE`. +### Example scenarios +- A reporting app might need `READ/WRITE`. +- A stand-up bot might only need `WRITE`.
6-6
: Add security caveat to “ALL”.Call out that “ALL” should be exceptional and likely requires admin approval/audit.
Consider adding:
- “Use ALL sparingly; request must be approved by an organization admin and audited regularly.”
docs/README.md (9)
2-4
: Rename top heading; avoid leading slash.“# /docs” looks like a path, not a title.
-# /docs +# Docs
8-9
: Clarify link-behavior sentence.Current phrasing is verbose; tighten and clarify where links might break.
-- The Markdown (.md) pages in this directory should be self-contained, including any static images or diagrams. However, please note that links within these documents may not function correctly, as they are relative to the `wireapp/wire-docs/src` directory. To ensure that document links work as intended, collaborate with the `wire-docs` repository to identify their correct locations. +- Markdown pages here should be self-contained (including images/diagrams). Links may not work if they assume `wireapp/wire-docs/src` paths. Coordinate with `wire-docs` to set correct targets.
19-19
: Add descriptive alt text for accessibility.“build” is not descriptive.
- +
21-26
: Section capitalization consistency and clarity.Make headings consistent and concise.
-# Documentation Management for Wire +# Documentation management for Wire
31-35
: Use ordered-list style consistently and end steps with periods.Minor readability improvement.
-1. Make changes directly in the `wire-apps-jvm-sdk/docs` directory -2. Create a PR against the `wire-apps-jvm-sdk:main` branch -3. Once merged, these changes will be picked up by `wire-docs` through submodule updates +1. Make changes directly in `wire-apps-jvm-sdk/docs`. +2. Create a PR targeting `wire-apps-jvm-sdk:main`. +3. After merge, `wire-docs` picks them up via submodule updates.
40-47
: Clarify move flow and ensure link durability; fix fenced code language requirement later.Also, linking to anchors like mkdocs.yml#L9 is brittle.
- Add a note to replace line-anchored URLs with stable anchors or section links in
mkdocs.yml
.- Confirm the relative path layout in
wire-docs
before publishing (see next comment with commands).
71-84
: Same MD026 issue for “Removing files” subheadings; standardize step punctuation.Also keep steps parallel between repos.
-### Removing files from wire-apps-jvm-sdk/docs: +### Removing files from wire-apps-jvm-sdk/docs @@ -### Removing files from wire-docs: +### Removing files from wire-docs
85-88
: Clarify submodule update responsibility.State who/what updates the submodule and when, to avoid confusion.
-All changes to documentation go live when the [CI Build](https://github.com/wireapp/wire-docs/blob/main/.github/workflows/build.yaml) workflow on `wire-docs` runs. Currently, submodule updates from `wire-apps-jvm-sdk` to `wire-docs` are performed manually, but there are plans to automate this with a pipeline. +All changes go live when the [CI Build](https://github.com/wireapp/wire-docs/blob/main/.github/workflows/build.yaml) workflow on `wire-docs` runs. Today, updating the `wire-apps-jvm-sdk` submodule in `wire-docs` is manual; automation is planned.
6-6
: Image guidance: prefer lightweight SVG, source files, and repo size hygiene.To address concerns about repo bloat and keep docs close to code:
- Keep images as optimized SVGs (use SVGO). Commit editable sources (e.g., .drawio) alongside exports.
- If images become large or numerous, move them to wire-docs and reference them from here, or host via GitHub Pages artifacts.
Also applies to: 19-19
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
docs/build.svg
is excluded by!**/*.svg
docs/diagram.svg
is excluded by!**/*.svg
docs/src/images/create-app.png
is excluded by!**/*.png
📒 Files selected for processing (4)
docs/README.md
(1 hunks)docs/src/README.md
(1 hunks)docs/src/creating-an-app.md
(1 hunks)docs/src/permissions.md
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/src/creating-an-app.md
- docs/src/README.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/permissions.md
5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
7-7: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
9-9: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
docs/README.md
11-11: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
17-17: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
57-57: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
66-66: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
73-73: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
79-79: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ 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)
48-52
: Specify bash code fence, idempotent flags, and verify symlink path
- Change the fenced block to use “```bash” and update to
rm -f creating-an-app.md
and `ln -sf /creating-an-app.md creating-an-app.md`.- Confirm that the
docs/src/integrations
directory actually exists and that the target file location (e.g. within the wire-apps-jvm-sdk module) matches the relative path; adjust as needed.
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764
The PR Description
What's new in this PR?
Issues
There was no sample documentation on how to create an app from scratch (high-level) as in onboarding the app from Team Settings.
Solutions
Add .md files inside
docs/apps/.
Notes (Optional)
This is just a sample as not everything is fully complete (both documentation and code).
This will be used to display in https://github.com/wireapp/wire-docs
Summary by CodeRabbit