Skip to content

chore: clean up docs and remove dead migration code#52

Merged
StuMason merged 1 commit intomainfrom
chore/docs-cleanup
Jan 23, 2026
Merged

chore: clean up docs and remove dead migration code#52
StuMason merged 1 commit intomainfrom
chore/docs-cleanup

Conversation

@StuMason
Copy link
Owner

Summary

  • Remove all references to non-existent coolify_resources table
  • Remove dead migration code from InstallCommand and ServiceProvider
  • Update docs to reflect environment-based resource fetching (not git repo matching)
  • Rewrite CHANGELOG with accurate v3.0.0 and [Unreleased] sections
  • Document environment switcher functionality in multi-app.md

Test plan

  • composer test passes (230 tests)
  • composer lint passes

- Remove references to non-existent coolify_resources table
- Remove dead migration code from InstallCommand and ServiceProvider
- Update docs to reflect environment-based resource fetching
- Rewrite CHANGELOG with accurate v3.0.0 and unreleased sections
- Document environment switcher in multi-app.md
@StuMason StuMason merged commit cd224f9 into main Jan 23, 2026
7 checks passed
@claude
Copy link

claude bot commented Jan 23, 2026

Code Review

Summary

This is a well-executed cleanup PR that removes dead code and aligns documentation with the actual implementation. The changes are focused, clear, and improve code maintainability.

✅ Strengths

1. Code Quality

  • Clean removal of dead migration code that referenced a non-existent coolify_resources table
  • Proper cleanup from both InstallCommand (removed 16 lines) and CoolifyServiceProvider (removed 4 lines)
  • No orphaned imports left behind (Illuminate\Support\Facades\Schema correctly removed from InstallCommand.php:8)

2. Documentation Improvements

  • CHANGELOG properly restructured with accurate version history
  • [Unreleased] section clearly documents the environment-based architecture
  • Multi-app.md rewritten to focus on environment switcher functionality (which aligns with the actual dashboard implementation)
  • Consistent messaging across all docs about how application lookup works (environment-based vs git repo matching)

3. Testing

  • All 230 tests passing according to the PR description
  • Existing InstallCommandTest.php remains valid (doesn't test migration-related functionality)
  • No test updates needed since the removed code was never actually used

4. Best Practices

  • Follows repository conventions from CLAUDE.md
  • Commit message follows conventional commits format (chore:)
  • Clear PR description with test plan

🔍 Observations

1. Consistency Check
I verified that coolify_resources table references are completely removed:

# Only remaining reference is in CHANGELOG.md (documenting the removal)
grep -r "coolify_resources" --include="*.php"
# Result: No matches

2. Documentation Accuracy
The updated docs correctly reflect the environment-based resource fetching:

  • README.md: Updated to mention environment-based fetching
  • multi-app.md: Rewritten to document environment switcher
  • configuration.md: Explains environment selection logic
  • All references to git repo matching removed

3. No Breaking Changes

  • coolify:install command signature unchanged
  • All published tags remain the same (removed coolify-migrations which wasn't being used)
  • ServiceProvider boot/register flow unchanged

💡 Suggestions (Optional)

1. CHANGELOG Date
The v3.0.0 is dated 2026-01-22 (future date). If this hasn't been released yet, consider moving those entries to [Unreleased] section.

2. Installation Documentation
In installation.md:20, the comment still mentions:

- Publish dashboard assets

Consider updating to:

This will:
- Publish `config/coolify.php`
- Publish dashboard assets to `public/vendor/coolify/`
- Generate Dockerfile and docker configs
- Configure TrustProxies middleware

🔒 Security

No security concerns. This PR only removes dead code and updates documentation.

⚡ Performance

No performance impact. The removed code wasn't being executed anyway.

🧪 Test Coverage

Existing test coverage is adequate. The removed code:

  • Was never called (dead code)
  • Wasn't tested in the test suite
  • References a table that never existed

✨ Verdict

LGTM - This is a clean maintenance PR that improves code quality and documentation accuracy. The removal of dead migration code is safe and well-documented in the CHANGELOG. Ready to merge once the optional CHANGELOG date suggestion is addressed (if applicable).

Great work on the cleanup! 🎉

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.

1 participant