Add release notes for 5.0.0 - #1978
Conversation
|
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBumps docs to 5.0.0: adds 5.0.0 release notes and 5.x breaking-changes, removes many 4.x release-note files, updates Sphinx metadata and support matrix, migrates publishing blacklist to pathspecs, renames publish-related permissions/APIs, and simplifies upgrade guides and script names to .sh. ChangesRelease & upgrade docs (5.0.0)
Sequence Diagram(s)sequenceDiagram
participant Admin as System Admin
participant Archive as New Binary Archive
participant Install as Current Install
participant Scripts as Upgrade Scripts
participant Services as Server/Services
participant Logger as StudioUpgradeManagerImpl
Admin->>Archive: optionally extract/download new 5.0.0 bundle
Admin->>Install: stop services (Tomcat / Deployer)
Admin->>Scripts: run `upgrade-target.sh` / `start-upgrade.sh`
Scripts->>Install: migrate binaries/configs, run preflight
Scripts->>Services: execute `post-upgrade.sh` (rebuild indices, repo adjustments)
Services->>Logger: "Check for upgrades ..." log lines
Logger->>Admin: report completion/status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
source/support.rst (1)
25-29: Replace TBDs with actual release info before merge.
Fill “Released” and “Latest” cells once finalized to avoid publishing placeholders.If helpful, I can wire these values to a single place (e.g., conf.py) to avoid future drift. Want me to propose that change?
source/conf.py (1)
154-154: Add 5 to the versions switcher.To surface 5.x explicitly in the footer menu, include “5” alongside “current.”
- 'versions': ['current', '4', '4.1', '4.0', '3.1'], + 'versions': ['current', '5', '4', '4.1', '4.0', '3.1'],source/release-notes/5-x-breaking-changes.rst (2)
20-21: Call out migration guidance for removed Spring profile.Add a short note on what replaces
crafter_studio_externalDbor link to the setup page for external DB configuration in 5.x.
56-73: Cross-link to the authoritative configuration section.Add a reference to the “Publishing Blacklist” section so readers can jump to details.
- - Publishing blacklist configuration has been changed. |br| + - Publishing blacklist configuration has been changed. See :ref:`publishing-blacklist`. |br|source/release-notes/5-0-0.rst (1)
124-128: Deduplicate entry for PackageDetailsDialog packageId type.The same change is listed twice.
- - ``PackageDetailsDialog``: Updated ``packageId`` prop to be of type ``number``. ... - - ``PackageDetailsDialog``: Update ``packageId`` prop to be of type ``number``. + - ``PackageDetailsDialog``: Updated ``packageId`` prop to be of type ``number``.source/release-notes/index.rst (1)
9-15: Include a pointer to 5.x breaking changes.Add a short line under the intro or right after the 5.0.0 include to guide readers to the breaking changes page.
.. include:: /release-notes/5-0-0.rst + +.. note:: See also :ref:`breaking-changes-5-x` for 5.x breaking changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
source/by-role/system-admin/upgrade/upgrading-server.rst(1 hunks)source/conf.py(3 hunks)source/contribute/acknowledgements.rst(1 hunks)source/index.rst(2 hunks)source/reference/modules/studio.rst(2 hunks)source/release-notes/4-2-0.rst(0 hunks)source/release-notes/4-2-2.rst(0 hunks)source/release-notes/4-3-0.rst(0 hunks)source/release-notes/4-3-1.rst(0 hunks)source/release-notes/4-3-2.rst(0 hunks)source/release-notes/4-4-0.rst(0 hunks)source/release-notes/4-4-1.rst(0 hunks)source/release-notes/4-4-2.rst(0 hunks)source/release-notes/4-4-3.rst(0 hunks)source/release-notes/4-4-4.rst(0 hunks)source/release-notes/4-x-breaking-changes.rst(0 hunks)source/release-notes/5-0-0.rst(1 hunks)source/release-notes/5-x-breaking-changes.rst(1 hunks)source/release-notes/index.rst(1 hunks)source/support.rst(2 hunks)
💤 Files with no reviewable changes (11)
- source/release-notes/4-3-0.rst
- source/release-notes/4-4-1.rst
- source/release-notes/4-4-2.rst
- source/release-notes/4-3-1.rst
- source/release-notes/4-2-2.rst
- source/release-notes/4-4-3.rst
- source/release-notes/4-4-4.rst
- source/release-notes/4-3-2.rst
- source/release-notes/4-4-0.rst
- source/release-notes/4-2-0.rst
- source/release-notes/4-x-breaking-changes.rst
🔇 Additional comments (11)
source/contribute/acknowledgements.rst (1)
2-2: LGTM on metadata bump to 5.0.0.
Matches the 5.0.0 docs alignment.source/by-role/system-admin/upgrade/upgrading-server.rst (1)
68-68: Tighten wording; anchor verified
Avoid the duplicated "to" — apply the suggested rephrase. The :ref:breaking changes <breaking-changes-5-x>anchor exists at source/release-notes/5-x-breaking-changes.rst:5.source/support.rst (1)
2-2: LGTM on last-updated 5.0.0.
Consistent with the 5.0.0 docs update.source/reference/modules/studio.rst (1)
2-2: Version bump looks good.No further action.
source/conf.py (2)
32-32: Release set to 5.0.0 — OK.Matches the PR intent and extlinks usage.
296-303: Javadoc base/version linkage — OK.
extlinks['javadoc_base_url']correctly derives fromrelease.source/release-notes/5-0-0.rst (2)
7-7: Replace “Released TBD” with the actual release date before publishing.This blocks release.
171-173: Verify replacement endpoints (duplicated target).“Removed
workflow/publishandworkflow/requestPublish, replaced bypublishing/publishandpublishing/publish” has a duplicated replacement. Confirm the second should be a different endpoint (e.g.,publishing/calculateorworkflow/approve), then correct.source/index.rst (1)
78-78: Release blurb OK.Matches the 5.0.0 update elsewhere.
source/release-notes/index.rst (2)
2-2: Version stamp OK.No issues.
18-21: Cross-version link looks good.Linking to “4 Release Notes” via docs_base_url is consistent with extlinks config.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
source/index.rst (1)
2-2: Front-matter version bump looks correct; prior merge conflict resolved.
Last-updated now reads 5.0.0 and no conflict markers remain.
🧹 Nitpick comments (1)
source/index.rst (1)
80-80: Pin Docker tag to 5.0.0 for reproducibility.
Using :latest can drift after future releases.- docker run -p 8080:8080 craftercms/authoring_local:latest + docker run -p 8080:8080 craftercms/authoring_local:5.0.0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
source/index.rst(2 hunks)source/release-notes/5-0-0.rst(1 hunks)source/release-notes/5-x-breaking-changes.rst(1 hunks)source/support.rst(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- source/support.rst
- source/release-notes/5-x-breaking-changes.rst
🔇 Additional comments (4)
source/release-notes/5-0-0.rst (4)
7-7: Replace “Released TBD” with the actual release date before merging.
Avoid shipping placeholders in published docs.-Released *TBD* +Released *YYYY-MM-DD* <!-- replace with the actual 5.0.0 release date -->
23-44: Validate API doc anchors/links resolve.
Several :base_url: links depend on generated anchors; case or path mismatches will 404 in built docs. Please run link checks before merging.Would you confirm these with your Sphinx linkcheck job (or locally with “make linkcheck”) and fix any broken anchors?
171-171: Double-check replacement services text.
Both removed endpoints are listed as replaced by the samepublishing/publish. If that’s intentional, add a brief note; if not, correct the second replacement (perhapspublishing/requestPublish?).
220-225: Sanity-check external references before publishing.
Verify the GitHub project link for “5.0.0” and the cross-refcompatibility-with-3.1.xstill exist after the 5.x doc reorg.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
source/release-notes/5-0-0.rst (3)
117-117: Typo: "by" → "with" for consistency with correct prop replacement.The sentence mentions replacing the
stylesprop "Replaced bysxsprop." but this should be "Replaced withsxsprop." for grammatical consistency.- - Removed ``styles`` prop from ``ErrorState``, ``ApiResponseErrorState``, ``BlueprintForm``, ``LoadingState``, ``ConfirmDialog``, ``AlertDialog``, ``LauncherSection``, ``AceEditor``, ``ItemDisplay``, ``PasswordRequirementsDisplay``, ``PublishDialogForm``, ``ItemStateIcon``, ``ResizeableDrawer``, ``GlobalAppToolbar``, ``ViewToolbar``, ``PublishingStatusAvatar``, ``ItemPublishingTargetIcon`` and ``EmptyState``. Replaced by ``sxs`` prop. + - Removed ``styles`` prop from ``ErrorState``, ``ApiResponseErrorState``, ``BlueprintForm``, ``LoadingState``, ``ConfirmDialog``, ``AlertDialog``, ``LauncherSection``, ``AceEditor``, ``ItemDisplay``, ``PasswordRequirementsDisplay``, ``PublishDialogForm``, ``ItemStateIcon``, ``ResizeableDrawer``, ``GlobalAppToolbar``, ``ViewToolbar``, ``PublishingStatusAvatar``, ``ItemPublishingTargetIcon`` and ``EmptyState``. Replaced with ``sxs`` prop.
137-137: Missing closing backticks for prop name formatting.The prop name
onContentTypeSelectedis missing proper RST inline code formatting with closing backticks.- - Prop ``onContentTypeSelected`` changed its signature from sending an object with ``authoringBase``, ``path``, ``isNewContent``, ``contentTypeId``, ``onSaveSuccess`` to ``{ path: string; contentType: ContentType }`` + - Prop ``onContentTypeSelected`` changed its signature from sending an object with ``authoringBase``, ``path``, ``isNewContent``, ``contentTypeId``, ``onSaveSuccess`` to ``{ path: string; contentType: ContentType }``
189-189: Fix unbalanced backticks for inline code formatting.The
itemsproperty has unbalanced backticks that will render incorrectly in RST.- - ``items`` is now of type ``PublishingItem[]``. + - ``items`` is now of type ``PublishingItem[]``.
🧹 Nitpick comments (1)
source/release-notes/5-0-0.rst (1)
7-7: Update TBD placeholder with actual release date.The release date is marked as "TBD" (To Be Determined). Since this is a release notes document, consider updating this with the actual release date when available.
-Released *TBD* +Released *[ACTUAL_RELEASE_DATE]*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
source/release-notes/5-0-0.rst(1 hunks)
🔇 Additional comments (1)
source/release-notes/5-0-0.rst (1)
1-228: Overall document structure and content are comprehensive.The release notes document is well-structured and comprehensive, covering all major areas including new features, enhancements, UI changes, and bug fixes. The categorization is clear and the technical details are appropriately detailed for a major version release.
|
Update new APIs in list and removed APIs commits: 880fc37 3a64d54 76c14de |
|
commit: 7e8200f |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
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)
source/by-role/developer/common/adding-a-new-language.rst (1)
73-81:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winArray should include 'ko' instead of 'kr' after localeCode replacement.
The
localeCode.replace('kr', 'ko')on line 74 converts 'kr' to 'ko', but the subsequent array check on line 78 still includes 'kr' instead of 'ko'. This means that when a user with Korean locale ('kr') loads the UI, the code will:
- Replace 'kr' with 'ko' (line 74)
- Check if 'ko' is in
['de', 'es', 'kr', 'ja'](line 78)- Fail the check because 'ko' is not in the array
This will prevent Korean translations from being fetched.
🔧 Proposed fix
async function createIntlInstance(localeCode: string): Promise<IntlShape> { localeCode = localeCode.replace('kr', 'ko'); if ( !fetchedLocales[localeCode] && // Nothing to fetch point if we don't have the locale - ['de', 'es', 'kr', 'ja'].includes(localeCode) + ['de', 'es', 'ko', 'ja'].includes(localeCode) ) { let fetchedTranslations = await fetchLocale(localeCode as BundledLocaleCodes); ...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/by-role/developer/common/adding-a-new-language.rst` around lines 73 - 81, In createIntlInstance, after you normalize localeCode with localeCode.replace('kr','ko'), update the hard-coded allowed-locale array used in the fetchedLocales check to include 'ko' instead of 'kr' (e.g., change ['de','es','kr','ja'] to ['de','es','ko','ja']) so that the subsequent condition correctly triggers fetchLocale(localeCode as BundledLocaleCodes) for Korean; ensure you update the array referenced in the fetchedLocales check near the fetchedTranslations call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@source/by-role/developer/common/adding-a-new-language.rst`:
- Around line 73-81: In createIntlInstance, after you normalize localeCode with
localeCode.replace('kr','ko'), update the hard-coded allowed-locale array used
in the fetchedLocales check to include 'ko' instead of 'kr' (e.g., change
['de','es','kr','ja'] to ['de','es','ko','ja']) so that the subsequent condition
correctly triggers fetchLocale(localeCode as BundledLocaleCodes) for Korean;
ensure you update the array referenced in the fetchedLocales check near the
fetchedTranslations call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6328cf22-9005-4cc8-aab7-fb3fbc6261d1
📒 Files selected for processing (14)
source/by-role/developer/common/adding-a-new-language.rstsource/by-role/developer/upgrade/index.rstsource/by-role/system-admin/upgrade/index.rstsource/by-role/system-admin/upgrade/upgrading-cluster.rstsource/by-role/system-admin/upgrade/upgrading-search.rstsource/by-role/system-admin/upgrade/upgrading-server.rstsource/contribute/acknowledgements.rstsource/index.rstsource/reference/modules/engine.rstsource/reference/modules/studio.rstsource/release-notes/5-0-0.rstsource/release-notes/5-x-breaking-changes.rstsource/release-notes/index.rstsource/support.rst
✅ Files skipped from review due to trivial changes (6)
- source/by-role/developer/upgrade/index.rst
- source/contribute/acknowledgements.rst
- source/index.rst
- source/release-notes/5-x-breaking-changes.rst
- source/release-notes/5-0-0.rst
- source/release-notes/index.rst
🚧 Files skipped from review as they are similar to previous changes (7)
- source/reference/modules/engine.rst
- source/by-role/system-admin/upgrade/index.rst
- source/by-role/system-admin/upgrade/upgrading-cluster.rst
- source/by-role/system-admin/upgrade/upgrading-server.rst
- source/by-role/system-admin/upgrade/upgrading-search.rst
- source/reference/modules/studio.rst
- source/support.rst
|
commit f024528: Add new item order API to release notes craftersoftware/craftercms#8731 |
|
@alhambrav, this seems to have become more than release notes. I see other things in there. I think it's best if the new Form Engine stuff and non-release-note items flow in their own PR with a "hold" for the v5 release tag/note and are kept as a draft rather than mixing concerns. |
Ticket reference or full description of what's in the PR
Add release notes for 5.0.0