Skip to content

[PM-38986] [BEEEP] Update DC docs#817

Merged
BTreston merged 3 commits into
mainfrom
BEEEP-update-DC-docs
Jun 15, 2026
Merged

[PM-38986] [BEEEP] Update DC docs#817
BTreston merged 3 commits into
mainfrom
BEEEP-update-DC-docs

Conversation

@BTreston

Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-38986

📔 Objective

Updates documentation re: DC after the recent modernization has concluded.

📸 Screenshots

@BTreston BTreston changed the title BEEEP update DC docs [PM-38986] [BEEEP] Update DC docs Jun 12, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 12, 2026

Copy link
Copy Markdown

Deploying contributing-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: af968ee
Status: ✅  Deploy successful!
Preview URL: https://b7a01e91.contributing-docs.pages.dev
Branch Preview URL: https://beeep-update-dc-docs.contributing-docs.pages.dev

View logs

@BTreston BTreston marked this pull request as ready for review June 12, 2026 17:00
@BTreston BTreston requested a review from a team as a code owner June 12, 2026 17:00
@BTreston BTreston requested a review from eliykat June 12, 2026 17:00
@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: REQUEST CHANGES

Reviewed documentation updates for the Directory Connector after the recent modernization. Changes cover the new repository layout, Rust native module build step, Node.js version bump, and trailing-slash cleanup in OpenLDAP path references. The intent is clear, but one procedural step in open-ldap.md now contradicts the following step, and the Node.js version is out of step with the rest of the contributing docs.

Code Review Details
  • ⚠️ : "Start the OpenLDAP Docker container" appears in both step 2 and step 3 with different commands — the procedure now contradicts itself
    • docs/getting-started/business/directory-connector/open-ldap.md:26
  • 🎨 : Heading "Repository Structure" should be "Repository structure" per the project style guide (only the first word is capitalized)
    • docs/getting-started/business/directory-connector/index.mdx:9
  • ❓ : Node.js bumped from v20 (LTS) to v25 (non-LTS). The repo-wide tools doc recommends v22; please confirm v25 is what the DC repo's engines/.nvmrc actually pins
    • docs/getting-started/business/directory-connector/index.mdx:20

## Requirements

- [Node.js](https://nodejs.org/) v20
- [Node.js](https://nodejs.org/) v25

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.

QUESTION: Is Node.js v25 intentional? The repo-wide tools doc recommends v22 (LTS), and v25 is an odd-numbered non-LTS release.

Details

docs/getting-started/tools/index.md:38 lists the standard as NodeJS v22. Bumping Directory Connector from v20 (LTS) to v25 (non-LTS, current) is a departure from that baseline. If the DC repo's package.json engines field or .nvmrc actually pins v25, this is correct — otherwise this may have been intended as v22 or v24 (the current LTS lines).

@BTreston BTreston Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is intentional. V25 allows us to use node SEA builds. We will upgrade to a LTS version when one is available. V26 was recent released (as of May 5). So we can upgrade and update the docs here at our convenience.

Comment thread docs/getting-started/business/directory-connector/index.mdx
Comment thread docs/getting-started/business/directory-connector/open-ldap.md Outdated
@BTreston BTreston merged commit 6cd8a17 into main Jun 15, 2026
21 checks passed
@BTreston BTreston deleted the BEEEP-update-DC-docs branch June 15, 2026 15:05
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.

2 participants