Skip to content

Bug à l'installation avec mariadb (et non mysql)#544

Merged
dbarzin merged 1 commit intodbarzin:mainfrom
herewithme:patch-1
Nov 23, 2025
Merged

Bug à l'installation avec mariadb (et non mysql)#544
dbarzin merged 1 commit intodbarzin:mainfrom
herewithme:patch-1

Conversation

@herewithme
Copy link
Copy Markdown
Contributor

@herewithme herewithme commented Nov 23, 2025

Fix mariadb usage with 2024_06_27_123923_add_control_measure_table.php

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved database compatibility to consistently support both MySQL and MariaDB systems.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Nov 23, 2025

Walkthrough

A database migration file's driver check is expanded to treat both MySQL and MariaDB identically when determining which foreign keys to drop from the controls table.

Changes

Cohort / File(s) Summary
Database Driver Compatibility Fix
database/migrations/2024_06_27_123923_add_control_measure_table.php
Expands the database driver condition to include MariaDB alongside MySQL (DB::getDriverName() == 'mysql' || DB::getDriverName() == 'mariadb') for consistent foreign key drop behavior across compatible database systems

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single file modification with a straightforward logical operator expansion
  • Clear intent to align MySQL and MariaDB handling
  • Low risk change with minimal logic density

Poem

A rabbit hops through the database row,
"MariaDB and MySQL, now you both shall know—
Same path, same logic, together you'll go!" 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: fixing a MariaDB installation bug by treating MariaDB and MySQL as the same database driver group in migrations.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
database/migrations/2024_06_27_123923_add_control_measure_table.php (1)

67-67: Typo in comment.

"Wokrs" should be "Works".

-            // Wokrs only if DB is empty
+            // Works only if DB is empty
🧹 Nitpick comments (1)
database/migrations/2024_06_27_123923_add_control_measure_table.php (1)

29-29: Approved: Fix correctly handles MariaDB driver.

This change properly addresses the MariaDB installation bug by treating MariaDB the same as MySQL for foreign key operations, which is correct since Laravel reports MariaDB connections with driver name 'mariadb'.

Consider using === instead of == for type-safe string comparison:

-            if (DB::getDriverName() == 'mysql' || DB::getDriverName() == 'mariadb')
+            if (DB::getDriverName() === 'mysql' || DB::getDriverName() === 'mariadb')
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 6e7f43e and 6177da8.

📒 Files selected for processing (1)
  • database/migrations/2024_06_27_123923_add_control_measure_table.php (1 hunks)

@dbarzin dbarzin merged commit 45bd8e8 into dbarzin:main Nov 23, 2025
1 check passed
@dbarzin
Copy link
Copy Markdown
Owner

dbarzin commented Nov 23, 2025

Merci !

@herewithme
Copy link
Copy Markdown
Contributor Author

@dbarzin je suis repassé sur "mysql" post-installation, car j'avais d'autres erreurs dans l'interface.

Initialement, j'ai changé la valeur de "mysql" à "mariadb" parce que j'avais une erreur à l'installation à cause de l'avertissement :

mysql: Deprecated program name. It will be removed in a future release, use '/usr/bin/mariadb' instead

A méditer ;)

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