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:
Summary by CodeRabbit
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)
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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 |
|
Update upgrading sections, commit: b399ebe for craftercms/craftercms#8326 |
|
Add updated dependencies and update breaking changes for removed OpenSearch method |
|
Commit ab19ea9 and 0a7b1e7 Add note that getDescriptor is now removed craftercms/craftercms#8294 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/reference/modules/studio.rst (1)
3356-3433: Update publishing blacklist section to consistently use pathspecs (not regex).The example and surrounding prose reference the deprecated
studio.configuration.publishing.blacklist.regexproperty, which was replaced bystudio.configuration.publishing.blacklist.pathspecsin CrafterCMS 5.0.0 (see breaking changes documentation). The main configuration block correctly shows the new property, but the example code uses the old property—users copying the example will get non-functional configuration.Update all references across the section:
- Replace "regex" terminology with "pathspecs" in prose and comments
- Change
studio.configuration.publishing.blacklist.regextostudio.configuration.publishing.blacklist.pathspecsin the example- Update log messages to reference "pathspecs" instead of "regex patterns"
- Convert the example regex pattern to git pathspec syntax
Proposed diff
-A comma-separated list of regexes is used to configure items that should not be published. +A list of git pathspecs is used to configure items that should not be published. -Add the following lines with the regex for the item you wish not to be published. By default, ``.keep`` files are not published by CrafterCMS. Just add a ``,`` then your regex after ``.*/\.keep``: +Add the following line(s) with the pathspec(s) for the items you wish not to be published. By default, ``.keep`` files are not published by CrafterCMS. -[DEBUG] 2021-04-22T08:16:01,023 [studio.clockTaskExecutor-42] [deployment.PublishingManagerImpl] | File /static-assets/css/.keep of the site mysite will not be published because it matches the configured publishing blacklist regex patterns. +[DEBUG] 2021-04-22T08:16:01,023 [studio.clockTaskExecutor-42] [deployment.PublishingManagerImpl] | File /static-assets/css/.keep of the site mysite will not be published because it matches the configured publishing blacklist pathspecs. -Say, you do not want files under ``/static-assets/images/mytempimages`` to be published when a user performs a bulk publish or *Approve & Publish* of multiple items from the dashboard. We'll add to the ``studio.configuration.publishing.blacklist.regex`` the regex for items under ``/static-assets/images/mytempimages`` +Say, you do not want files under ``/static-assets/images/mytempimages`` to be published when a user performs a bulk publish or *Approve & Publish* of multiple items from the dashboard. We'll add a pathspec for items under ``/static-assets/images/mytempimages`` -.. code-block:: yaml - :caption: *studio-config-override.yaml* - - # Publishing blacklist configuration, items matching regexes on this list will never be published - studio.configuration.publishing.blacklist.regex: >- - .*/\.keep,\/static-assets\/images\/mytempimages\/.* +.. code-block:: yaml + :caption: *studio-config-override.yaml* + + # Publishing blacklist configuration, items matching pathspecs on this list will never be published + studio.configuration.publishing.blacklist.pathspecs: "/static-assets/images/mytempimages/**" -[DEBUG] 2021-04-22T12:48:28,992 [studio.clockTaskExecutor-36] [deployment.PublishingManagerImpl] | File /static-assets/images/mytempimages/26072150271_848c0008f0_o.jpg of the site mysite will not be published because it matches the configured publishing blacklist regex patterns. +[DEBUG] 2021-04-22T12:48:28,992 [studio.clockTaskExecutor-36] [deployment.PublishingManagerImpl] | File /static-assets/images/mytempimages/26072150271_848c0008f0_o.jpg of the site mysite will not be published because it matches the configured publishing blacklist pathspecs.
🤖 Fix all issues with AI agents
In `@source/release-notes/5-x-breaking-changes.rst`:
- Around line 79-86: The wording claiming "the entire file will be commented
out" is misleading; update the documentation around controller.groovy and the
Upgrade Manager to state that the script's original body will be commented out
but an active logger.error(...) statement will be left at top-level so the
script still runs and logs an error; adjust the paragraph in the 79-86 section
(and the similar wording in 91-128) to match the example by explicitly
describing that only the original controller body is commented while a
logger.error line remains active for administrators to notice.
- Around line 59-77: The YAML example for the new publishing blacklist property
has an extra leading space on the line with
studio.configuration.publishing.blacklist.pathspecs which makes it misaligned
with the previous YAML block; fix it by removing the extra indentation so the
property line aligns exactly like the earlier code-block example (ensure the
line containing studio.configuration.publishing.blacklist.pathspecs: "" has the
same indentation level as the other top-level YAML keys in that section).
♻️ Duplicate comments (1)
source/release-notes/5-x-breaking-changes.rst (1)
140-141: Cross-ref depends on label existing at release time (content-processors-configuration).If that label isn’t in the same release-doc set at merge time, Sphinx will fail with an unresolved
:ref:. Please ensure the label lands before the 5.0.0 docs ship (or adjust the link to an existing target).
🧹 Nitpick comments (1)
source/support.rst (1)
24-28: LGTM! New 5.0 version entry is well-structured.The new table row for version 5.0 is correctly formatted with appropriate TBD placeholders (expected during release preparation) and proper RST link syntax.
Optional observation: The Version column includes a download link for 5.0 but not for other active versions like 4.4. If this is intentional to highlight the latest release, no change is needed. Otherwise, consider adding download links for other active versions for consistency.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
source/index.rstsource/reference/modules/engine.rstsource/reference/modules/studio.rstsource/release-notes/5-x-breaking-changes.rstsource/support.rst
🚧 Files skipped from review as they are similar to previous changes (2)
- source/index.rst
- source/reference/modules/engine.rst
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-08-08T14:42:29.830Z
Learnt from: alhambrav
Repo: craftercms/docs PR: 2044
File: source/by-role/developer/common/content-modeling/content-modeling.rst:699-704
Timestamp: 2025-08-08T14:42:29.830Z
Learning: In docs file source/by-role/developer/common/content-modeling/content-modeling.rst, the team wants to keep a reminder about the upcoming “Form Engine Controller” docs (tracked in GitHub issue `#8313`). For future reviews, avoid blocking on a TODO placeholder; suggest a non-breaking rst comment or a note admonition instead of an rst `.. todo` directive.
Applied to files:
source/support.rstsource/release-notes/5-x-breaking-changes.rst
📚 Learning: 2025-09-23T14:20:52.728Z
Learnt from: alhambrav
Repo: craftercms/docs PR: 2062
File: source/release-notes/4-5-0.rst:7-7
Timestamp: 2025-09-23T14:20:52.728Z
Learning: In CrafterCMS release notes files, "Released *TBD, 2025*" placeholders are intentionally used during release preparation phase. The actual release date is only set once the product is officially released, not during the preparation phase.
Applied to files:
source/support.rst
📚 Learning: 2025-08-21T14:50:04.744Z
Learnt from: alhambrav
Repo: craftercms/docs PR: 2049
File: source/by-role/system-admin/configuration.rst:671-673
Timestamp: 2025-08-21T14:50:04.744Z
Learning: In RST/Sphinx documentation, anchor tags usually appear before section marker characters (like `-----`), and explicit `.. _<label>:` declarations are not always required for section references to resolve correctly. References can resolve across multiple files in a documentation project based on section titles and implicit referencing.
Applied to files:
source/release-notes/5-x-breaking-changes.rst
📚 Learning: 2025-09-11T14:05:10.616Z
Learnt from: alhambrav
Repo: craftercms/docs PR: 1978
File: source/reference/modules/studio.rst:3370-3378
Timestamp: 2025-09-11T14:05:10.616Z
Learning: When reviewing CrafterCMS documentation, code blocks and configuration examples must accurately reflect the actual source code implementation, even if there appear to be inconsistencies in naming or approaches. The documentation should not be "cleaned up" to show ideal consistency if that would make it inaccurate relative to what the source code actually provides.
Applied to files:
source/release-notes/5-x-breaking-changes.rstsource/reference/modules/studio.rst
📚 Learning: 2025-09-02T20:40:32.613Z
Learnt from: alhambrav
Repo: craftercms/docs PR: 2042
File: source/reference/modules/studio.rst:0-0
Timestamp: 2025-09-02T20:40:32.613Z
Learning: When reviewing CrafterCMS Studio documentation, property names in configuration examples should match the actual Studio source code implementation, even if the underlying Git config keys are different. Studio may have a translation layer that maps Studio property names to actual Git configuration keys.
Applied to files:
source/reference/modules/studio.rst
🔇 Additional comments (1)
source/support.rst (1)
2-2: LGTM!The
last-updatedversion bump to 5.0.0 aligns with the PR objective of adding 5.0.0 release notes.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
source/reference/modules/studio.rst (1)
3389-3396:⚠️ Potential issue | 🟠 MajorPublishing blacklist docs are still internally inconsistent after the pathspecs update.
This block now documents
studio.configuration.publishing.blacklist.pathspecs, but nearby instructions/examples/log wording still describe regex andstudio.configuration.publishing.blacklist.regex, which can cause incorrect configuration guidance. Please align the section to one behavior/property set for 5.0.0.Based on learnings: documentation examples and property names should match the actual Studio implementation, even when naming differs from underlying Git concepts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/reference/modules/studio.rst` around lines 3389 - 3396, The documentation is inconsistent: some lines reference regex and studio.configuration.publishing.blacklist.regex while the current text and example use studio.configuration.publishing.blacklist.pathspecs; update this section to consistently document the pathspec-based behavior for 5.0.0 by replacing any mentions of "regex" and the property studio.configuration.publishing.blacklist.regex with studio.configuration.publishing.blacklist.pathspecs, adjust example entries and explanatory wording to describe git pathspec semantics (keeping the .keep note), and ensure example config and log/messages all use the same property name and pathspec terminology.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@source/release-notes/5-0-0.rst`:
- Line 187: Replace or remove the raw developer note "FE2 TODO:
image=>image-picker, text=>input, etc services/contentType typeMap removed" so
the release notes are user-facing: either rewrite that line into a clear release
note (e.g., describe that the services/contentType typeMap was removed and which
front-end component mappings changed such as image -> image-picker, text ->
input, with context and impact) or delete the line if the change is not ready to
document; update the exact literal TODO string accordingly.
- Line 195: The release note line incorrectly lists the same replacement twice;
update the sentence that says "Removed ``workflow/publish`` and
``workflow/requestPublish``, replaced by ``publishing/publish`` and
``publishing/publish``" so the second ``publishing/publish`` is replaced with
the correct service name (e.g., the replacement for
``workflow/requestPublish``); locate that text in source/release-notes/5-0-0.rst
and change the duplicate to the proper service identifier (verify the exact
replacement against the studio-ui source/changelog before committing).
- Around line 228-232: The two entries conflict: either drop the redundant
"Removed ``fetchDetailedItemComplete``" line or clarify that a distinct,
different action was removed; update the text so it consistently states that
``fetchDetailedItemComplete`` was renamed to ``fetchContentItemComplete``
(remove the "Removed" bullet) or, if there really was an additional removal,
change the second bullet to explicitly name the different removed action and
explain how it differs from the renamed
``fetchDetailedItemComplete``/``fetchContentItemComplete`` pair.
---
Duplicate comments:
In `@source/reference/modules/studio.rst`:
- Around line 3389-3396: The documentation is inconsistent: some lines reference
regex and studio.configuration.publishing.blacklist.regex while the current text
and example use studio.configuration.publishing.blacklist.pathspecs; update this
section to consistently document the pathspec-based behavior for 5.0.0 by
replacing any mentions of "regex" and the property
studio.configuration.publishing.blacklist.regex with
studio.configuration.publishing.blacklist.pathspecs, adjust example entries and
explanatory wording to describe git pathspec semantics (keeping the .keep note),
and ensure example config and log/messages all use the same property name and
pathspec terminology.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
source/_static/api/deployer.htmlsource/_static/api/studio.htmlsource/by-role/system-admin/upgrade/index.rstsource/contribute/acknowledgements.rstsource/index.rstsource/reference/modules/engine.rstsource/reference/modules/studio.rstsource/release-notes/5-0-0.rstsource/support.rst
🚧 Files skipped from review as they are similar to previous changes (3)
- source/index.rst
- source/reference/modules/engine.rst
- source/contribute/acknowledgements.rst
|
commit: 68122b2 |
|
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
2 participants
Ticket reference or full description of what's in the PR
Add release notes for 5.0.0