-
Notifications
You must be signed in to change notification settings - Fork 63
Update to version 7.0.0 #239
Conversation
📝 WalkthroughWalkthroughThis PR disables scheduled nightly cron triggers (keeping manual dispatch), updates the release workflow to use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks✅ Passed checks (1 passed)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
104-104: Specify the:legacytag explicitly in the legacy Quick Start example.Line 104 references
lycheeorg/lycheewithout a tag, which defaults to:latest(the new FrankenPHP-based image). Since this example is in the "Quick Start (Legacy)" section, it should explicitly use the:legacytag to avoid confusion.🔎 Proposed fix
-lycheeorg/lychee +lycheeorg/lychee:legacy
🧹 Nitpick comments (1)
README.md (1)
21-21: Optional: Consider hyphenating compound adjectives.Static analysis suggests using "bleeding-edge" (hyphenated) when used as a compound adjective modifying a noun, per standard English grammar conventions.
🔎 Suggested changes
-* `edge`: Development/bleeding edge builds (latest master commit) +* `edge`: Development/bleeding-edge builds (latest master commit)-* ~~`alpha`: Current alpha branch tag (The alpha branch contains bleeding edge changes that are not peer-reviewed)~~ +* ~~`alpha`: Current alpha branch tag (The alpha branch contains bleeding-edge changes that are not peer-reviewed)~~Also applies to: 56-56
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/nightly-alpha.yml.github/workflows/nightly.yml.github/workflows/release.ymlREADME.md
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~21-~21: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... (e.g., v7.0.0) * edge: Development/bleeding edge builds (latest master commit) **Legacy...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~27-~27: Ensure spelling is correct
Context: ... boot time drops from 40-60ms to just 4-6ms per request, with 3-4x better throughpu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~56-~56: Use a hyphen to join words.
Context: ... tag (The alpha branch contains bleeding edge changes that are not peer-reviewed)...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
.github/workflows/nightly-alpha.yml (1)
4-5: LGTM! Nightly schedule disabled as expected.Commenting out the cron schedule aligns with the repository deprecation while preserving the ability to trigger builds manually via
workflow_dispatch..github/workflows/nightly.yml (1)
4-5: LGTM! Consistent with nightly-alpha workflow.The cron schedule is properly disabled while maintaining manual trigger capability, consistent with the deprecation strategy.
README.md (1)
11-30: LGTM! Clear deprecation notice and migration guidance.The deprecation notice is prominent and provides helpful migration information, including the rationale for upgrading (performance improvements with FrankenPHP) and a link to the official upgrade guide.
There was a problem hiding this 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)
README.md (1)
104-104: Critical: Specify the:legacytag in the docker run example.Line 104 uses
lycheeorg/lycheewithout a tag, which defaults to:latest. According to the migration section (lines 18-19),:latestnow points to the new FrankenPHP-based image, not the legacy nginx+FPM image that this documentation describes. This will cause users to pull the wrong image.🔎 Proposed fix
--net network_name \ --link db_name \ -lycheeorg/lychee +lycheeorg/lychee:legacy
🧹 Nitpick comments (1)
README.md (1)
21-21: Consider hyphenating "bleeding edge" when used as a compound adjective.When "bleeding edge" modifies a noun (e.g., "bleeding-edge builds" or "bleeding-edge changes"), it should be hyphenated according to standard English grammar rules.
🔎 Proposed fix
-* `edge`: Development/bleeding edge builds (latest master commit) +* `edge`: Development/bleeding-edge builds (latest master commit)-* ~~`alpha`: Current alpha branch tag (The alpha branch contains bleeding edge changes that are not peer-reviewed)~~ +* ~~`alpha`: Current alpha branch tag (The alpha branch contains bleeding-edge changes that are not peer-reviewed)~~Also applies to: 56-56
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/release.ymlREADME.md
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~21-~21: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... (e.g., v7.0.0) * edge: Development/bleeding edge builds (latest master commit) **Legacy...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~27-~27: Ensure spelling is correct
Context: ... boot time drops from 40-60ms to just 4-6ms per request, with 3-4x better throughpu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~56-~56: Use a hyphen to join words.
Context: ... tag (The alpha branch contains bleeding edge changes that are not peer-reviewed)...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
README.md (3)
11-15: Clear deprecation notice.The deprecation notice is prominent and well-placed, clearly directing users to the new repository.
16-29: Comprehensive migration guidance.The migration section effectively explains the new tag structure, provides compelling performance metrics for the upgrade, and includes clear migration instructions with a link to the official guide.
61-65: LGTM!The Quick Start section correctly uses the
:legacytag and provides clear guidance about the legacy nature of these instructions..github/workflows/release.yml (1)
177-182: Previous issue has been resolved - workflow logic is now correct.The retag logic correctly implements the legacy tagging scheme:
- Tags the tested image with version-specific and generic legacy tags
- Copies to lychee-laravel repository with legacy suffix
- Tags the copied image as legacy
- Mirrors the legacy tag to ghcr.io
The previous review comment identified an issue where line 180 attempted to tag a non-existent image. This has been fixed: line 179 now creates
lycheeorg/lychee-laravel:${{ github.event.inputs.tagname }}-legacyviacrane cp, and line 180 correctly tags that newly created image aslegacy.
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.