Skip to content

Conversation

eliykat
Copy link
Member

@eliykat eliykat commented Oct 7, 2025

🎟️ Tracking

Internal discussions. See https://bitwarden.slack.com/archives/C065AUDER62/p1759499713149319.

📔 Objective

Update the database setup and integration test instructions to include MariaDB, which I added to docker compose and the migrate script in bitwarden/server#5816. The main changes are:

  • adding MariaDB as a tab item to the Getting Started -> EF page, with instructions
  • adding the example user secrets for integration testing
  • also updating the shared user secrets in our internal Bitwarden collection to match (already done)

Plus this PR in server, which fixes a few minor issues with the migrate script and docker compose: bitwarden/server#6420.

I also made some other updates while I was here:

  • remove some duplicate or unnecessary wording
  • add more context around using separate databases for integration tests
  • making instructions step-by-step rather than prose
  • better cross linking so you can find the other page in the Contributing section. (This division of responsibilities is a bit confusing but out of scope to revisit here.)
  • remove references to the deprecated [DatabaseTheory] attribute.

The current tooling here is not ideal and should be revisited. Also, using mysql as the key for MariaDB is confusing. However the focus of this PR is documenting what we currently have rather than changing it.

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation
    team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed
    issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

@eliykat eliykat requested a review from withinfocus October 7, 2025 09:44
@eliykat eliykat requested a review from a team as a code owner October 7, 2025 09:44
Copy link

cloudflare-workers-and-pages bot commented Oct 7, 2025

Deploying contributing-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: abbc4e0
Status: ✅  Deploy successful!
Preview URL: https://10b81921.contributing-docs.pages.dev
Branch Preview URL: https://mariadb.contributing-docs.pages.dev

View logs

Copy link

claude bot commented Oct 7, 2025

Claude finished @eliykat's task —— View job


PR Review: Add MariaDB instructions and clarify EF testing

Completed review of the documentation changes for accuracy, clarity, and formatting.

Review Summary

This PR successfully adds MariaDB support documentation and improves the overall clarity of database setup instructions. The changes are well-structured and follow the established documentation patterns.

✅ Positive Changes

📝 Content Quality:

  • Excellent addition of MariaDB as a new database option with clear instructions
  • Improved step-by-step formatting makes instructions much easier to follow
  • Good cross-referencing between the Getting Started and Contributing sections
  • Helpful clarification about using separate databases for integration testing

🎨 Structure & Style:

  • Proper use of tabs for different database providers
  • Consistent formatting with existing documentation style
  • Well-organized frontmatter and navigation structure
  • Clear separation between development and testing configurations

🔧 Technical Accuracy:

  • Correct connection string formats for all database types
  • Proper Docker Compose profile references (--profile mariadb)
  • Accurate migration command examples
  • Valid user secrets configuration format

🎯 Key Improvements Made

docs/getting-started/server/database/ef/index.mdx:

  • Added MariaDB as a new tab option with comprehensive setup instructions
  • Enhanced integration test configuration section with clearer examples
  • Improved explanation of database provider switching
  • Better organization of user secrets examples

docs/contributing/testing/database/index.md:

  • Removed deprecated [DatabaseTheory] references (good cleanup!)
  • Added better cross-linking to the EF setup page
  • Clearer explanation of the testing framework and database configuration
  • Improved formatting for better readability

📚 Documentation Standards Compliance

  • ✅ Follows numbered instruction format with action verbs
  • ✅ Uses proper code blocks (not inline code)
  • ✅ Maintains consistent heading capitalization
  • ✅ Includes appropriate warning and info callouts
  • ✅ Proper use of conditional content tags where needed

🏆 Overall Assessment

This is a high-quality documentation update that:

  • Successfully documents the new MariaDB support
  • Significantly improves the clarity and usability of database setup instructions
  • Maintains consistency with existing documentation standards
  • Provides comprehensive examples for both development and testing scenarios

The changes will make it much easier for developers to set up and test with different database providers, which aligns perfectly with Bitwarden's multi-database support strategy.

Recommendation: ✅ Approve

Copy link

github-actions bot commented Oct 7, 2025

Logo
Checkmarx One – Scan Summary & Details3fa68623-2f0e-43ef-87bf-c86505eabca4

Great job! No new security vulnerabilities introduced in this pull request

Copy link
Contributor

@withinfocus withinfocus left a comment

Choose a reason for hiding this comment

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

One ⛏️ because I just updated all the headings in this repo. Thanks for all these clarifications!


:::

### Run Migrations
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### Run Migrations
### Run migrations

Copy link

claude bot commented Oct 7, 2025

Claude finished @eliykat's task —— View job


PR Review: Add MariaDB instructions and clarify EF testing

I've completed a comprehensive review of the documentation changes for accuracy, clarity, and formatting compliance.

Todo List

  • Read repository guidelines from CLAUDE.md
  • Analyze changed files for content and structure
  • Review documentation for accuracy and clarity
  • Check adherence to style guide requirements
  • Provide comprehensive review feedback

Review Summary

This PR successfully adds MariaDB support documentation and improves the overall clarity of database setup instructions. The changes are well-structured and follow the established documentation patterns.

✅ Positive Changes

📝 Content Quality:

  • Excellent addition of MariaDB as a new database option with clear instructions
  • Improved step-by-step formatting makes instructions much easier to follow
  • Good cross-referencing between the Getting Started and Contributing sections
  • Helpful clarification about using separate databases for integration testing

🎨 Structure & Style:

  • Proper use of tabs for different database providers
  • Consistent formatting with existing documentation style
  • Well-organized frontmatter and navigation structure
  • Clear separation between development and testing configurations

🔧 Technical Accuracy:

  • Correct connection string formats for all database types
  • Proper Docker Compose profile references (--profile mariadb)
  • Accurate migration command examples
  • Valid user secrets configuration format

🎯 Key Improvements Made

docs/getting-started/server/database/ef/index.mdx:

  • Added MariaDB as a new tab option with comprehensive setup instructions
  • Enhanced integration test configuration section with clearer examples
  • Improved explanation of database provider switching
  • Better organization of user secrets examples
  • Fixed inconsistent formatting and removed redundant text

docs/contributing/testing/database/index.md:

  • ✅ Removed deprecated [DatabaseTheory] references (excellent cleanup!)
  • Added better cross-linking to the EF setup page
  • Clearer explanation of the testing framework and database configuration
  • Improved formatting for better readability
  • Fixed grammatical issues and sentence structure

📚 Documentation Standards Compliance

Checking against the CLAUDE.md style guide:

  • Numbered instructions: Uses proper action-oriented verbs ("confirm", "run", "add")
  • Heading capitalization: Follows "sentence case for headings" rule correctly
  • Code blocks: All commands properly formatted in code blocks (not inline)
  • Concise paragraphs: Content is to-the-point and well-structured
  • Consistent formatting: Maintains established patterns for frontmatter and structure
  • Cross-referencing: Good navigation between related sections

🔍 Notable Technical Details

MariaDB Integration:

  • Correctly explains that MariaDB uses mysql configuration keys (addresses the confusing tooling mentioned in PR description)
  • Proper Docker profile configuration (--profile mariadb)
  • Clear separation of MariaDB from MySQL in documentation while acknowledging shared configuration

Integration Testing:

  • Excellent improvement in explaining test database separation (vault_test vs vault_dev)
  • Better organization of user secrets with clear step-by-step instructions
  • Helpful migration command examples with different flags (-test, -all)

⚠️ One Minor Issue Noted

The existing review comment correctly identifies a heading capitalization issue:

  • Line 217 has "Run Migrations" but should be "Run migrations" to follow the sentence case guideline from CLAUDE.md

🏆 Overall Assessment

This is a high-quality documentation update that:

  • Successfully documents the new MariaDB support
  • Significantly improves the clarity and usability of database setup instructions
  • Maintains consistency with existing documentation standards
  • Provides comprehensive examples for both development and testing scenarios
  • Removes deprecated references and improves cross-linking

The changes will make it much easier for developers to set up and test with different database providers, which aligns perfectly with Bitwarden's multi-database support strategy.

Recommendation: ✅ Approve (pending the minor heading fix already noted in reviews)

@eliykat eliykat requested a review from withinfocus October 7, 2025 22:19
@eliykat eliykat merged commit c3b9421 into main Oct 8, 2025
12 checks passed
@eliykat eliykat deleted the mariadb branch October 8, 2025 20: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