Skip to content

Refactor port tables to use list-table format and improve clarity#9028

Open
hanzei wants to merge 5 commits into
masterfrom
claude/ports-documentation-ltgh37
Open

Refactor port tables to use list-table format and improve clarity#9028
hanzei wants to merge 5 commits into
masterfrom
claude/ports-documentation-ltgh37

Conversation

@hanzei

@hanzei hanzei commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Restructured the Mattermost services ports documentation to improve readability and maintainability by converting from RST grid tables to the more flexible list-table format, while also enhancing the content organization and clarity.

image

Key Changes

  • Table Format Migration: Converted all port reference tables from RST grid table format to list-table format for better maintainability and flexibility
  • Content Reorganization: Split the Mattermost Server ports table into two separate tables:
    • Inbound ports table (HTTP/WebSocket, Cluster, Metrics)
    • Outbound ports table (Database, LDAP, S3 Storage, SMTP, Push Notifications)
  • Improved Clarity:
    • Changed "Direction" column to "Notes" for more descriptive information
    • Enhanced notes with better explanations (e.g., "Both TCP and UDP must be open" for Cluster)
    • Formatted config settings with inline code formatting (backticks)
    • Clarified port usage context (e.g., "Ports 80 and 443 are typically used when running HTTPS")
  • Updated References: Changed "The following table" to "The following tables" in the introductory text to reflect the new structure
  • Push Proxy Table: Converted to list-table format with consistent structure and added "Inbound ports" section header

Implementation Details

  • All port information and configuration settings remain unchanged; this is purely a documentation structure improvement
  • The list-table format provides better readability in both source and rendered documentation
  • Separation of inbound and outbound ports makes it easier for system administrators to understand firewall configuration requirements

https://claude.ai/code/session_01KteMBCm721b5vmwTqyoxDZ

Replaces the single wide table (which caused horizontal scrolling) with
separate inbound and outbound list-tables for Mattermost Server and
Push Proxy. Drops the redundant Direction column since direction is now
conveyed by the section heading.
@github-actions

Copy link
Copy Markdown
Contributor

Newest code from mattermost has been published to preview environment for Git SHA 57ea8ed

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR corrects an apostrophe in the File Storage bullet and restructures the Mattermost services ports section: replacing grid tables with Sphinx list-table, splitting Mattermost Server ports into explicit Inbound and Outbound subsections, and updating the Push Proxy port entry.

Changes

Application Architecture Documentation

Layer / File(s) Summary
File storage section typography
source/deployment-guide/reference-architecture/application-architecture.rst
Updated "Local Storage" bullet to use standard ASCII apostrophe.
Mattermost services ports table restructuring
source/deployment-guide/reference-architecture/application-architecture.rst
Replaced grid-style port tables with Sphinx list-table markup. Mattermost Server ports now organized into Inbound and Outbound subsections with rows linking services to config settings, ports, protocols, and clarified direction/notes. Push Proxy port entry rewritten to note internal-only reachability from Mattermost Server nodes and requirement for self-hosted push proxy.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main changes: refactoring port tables to use list-table format and improving documentation clarity.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description accurately details the restructuring of port documentation tables from grid format to list-table format, with specific examples of content reorganization and clarity improvements that align with the actual changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/ports-documentation-ltgh37

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@source/deployment-guide/reference-architecture/application-architecture.rst`:
- Around line 169-174: Update the two listener entries ("Metrics" and "Push
Proxy") to explicitly state direction, protocol, source/destination scope, and
whether access is required: for Metrics (MetricsSettings.ListenAddress, port
8067) mark it as TCP, inbound to the application from internal/trusted
monitoring hosts only (internal-only, optional for external access), and for
Push Proxy (port 8066) mark it as TCP, inbound from trusted proxy servers or
internal components only (internal-only and required if using a proxy); apply
the same wording changes to the other occurrence referencing these listeners so
admins clearly understand the allowed source IPs, that these are not open to the
public internet, and whether enabling is mandatory or optional.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b3763b62-98c0-4baf-bbc5-b184d07160e6

📥 Commits

Reviewing files that changed from the base of the PR and between eb35cda and 57ea8ed.

📒 Files selected for processing (1)
  • source/deployment-guide/reference-architecture/application-architecture.rst

hanzei added 2 commits June 12, 2026 10:10
Metrics (8067) should only be reachable by trusted monitoring hosts
(e.g. Prometheus), not exposed broadly. Push Proxy (8066) should only
be reachable from Mattermost Server nodes.
@github-actions

Copy link
Copy Markdown
Contributor

Newest code from mattermost has been published to preview environment for Git SHA 637e3d8

@github-actions

Copy link
Copy Markdown
Contributor

Newest code from mattermost has been published to preview environment for Git SHA 46ffb44

@hanzei hanzei added the 2: Editor Review Requires review by an editor label Jun 12, 2026
@hanzei hanzei requested a review from connormullen-hub June 12, 2026 08:21
@hanzei hanzei added the 1: Dev Review Requires review by a core commiter label Jun 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Newest code from mattermost has been published to preview environment for Git SHA 5c86d86

@github-actions

Copy link
Copy Markdown
Contributor

Newest code from mattermost has been published to preview environment for Git SHA aa6d500

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1: Dev Review Requires review by a core commiter 2: Editor Review Requires review by an editor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant