Skip to content

Conversation

@trmartin4
Copy link
Member

@trmartin4 trmartin4 commented Jun 28, 2025

📔 Objective

As written, our SQL Code Style docs include mostly instructions on how to write migration scripts. This PR moves those best practices closer to where they're applicable - the database migration docs - and also formats the documentation with expanders for easier consumption.

Note that I left the SQL Code Style docs as-is. These will be expanded upon in #648, and I didn't want to cause merge conflicts there.

⏰ 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

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jun 28, 2025

Deploying contributing-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 15fae3c
Status: ✅  Deploy successful!
Preview URL: https://8fdf2aa3.contributing-docs.pages.dev
Branch Preview URL: https://restructure-sql-docs.contributing-docs.pages.dev

View logs

@github-actions
Copy link
Contributor

github-actions bot commented Jun 28, 2025

Logo
Checkmarx One – Scan Summary & Details3d776b19-1c58-4443-b97b-2e9645ed1253

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

ike-kottlowski
ike-kottlowski previously approved these changes Jul 1, 2025
@withinfocus
Copy link
Contributor

We got deep into database documentation at #593 and it has a bunch of moves. I'd like to get that merged first.

@withinfocus
Copy link
Contributor

My other organization-related PR has merged so should we take a fresh look here?

@trmartin4 trmartin4 changed the base branch from database-best-practice-addition to main August 16, 2025 01:36
@trmartin4 trmartin4 dismissed ike-kottlowski’s stale review August 16, 2025 01:36

The base branch was changed.

@trmartin4 trmartin4 changed the title Restructure-sql-docs Restructure database migration docs Aug 16, 2025
@trmartin4
Copy link
Member Author

@withinfocus I've updated these docs to incorporate your changes.

I also reached out to @mkincaid-bw because I think that his changes to the SQL Code Style doc will complement my changes very nicely; I've extracted all the migration-specific instructions into a separate doc, leaving the door open for more in-depth code style specific to SQL, which he's doing in #648.

Would it be preferable for me to undo the changes to the Code Style doc in my PR? I can do that before I open it for review, as what I put in there (https://restructure-sql-docs.contributing-docs.pages.dev/contributing/code-style/sql) is very basic and based on some recommendations given for one of our database changes.

@trmartin4 trmartin4 changed the title Restructure database migration docs Move database migration docs Aug 19, 2025
@trmartin4 trmartin4 marked this pull request as ready for review August 19, 2025 13:08
@trmartin4 trmartin4 requested a review from a team as a code owner August 19, 2025 13:09
@withinfocus
Copy link
Contributor

I feel like these changes are SQL coding standards, not migration-writing ones. We should think of column additions, view recreations, etc. as just those standards and not focus on a specific usage. It would be ideal to me to have this in one document.

@trmartin4 trmartin4 closed this Sep 6, 2025
@trmartin4 trmartin4 mentioned this pull request Sep 6, 2025
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.

4 participants