Release 8.0.0 #35999
Release 8.0.0 #35999
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 73d9200 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #35999 +/- ##
===========================================
+ Coverage 68.12% 68.44% +0.32%
===========================================
Files 3364 2877 -487
Lines 115811 101867 -13944
Branches 20901 17812 -3089
===========================================
- Hits 78891 69727 -9164
+ Misses 34224 30493 -3731
+ Partials 2696 1647 -1049
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| export const notifyOnLivechatPriorityChanged = async ( | ||
| data: Pick<ILivechatPriority, 'name' | '_id'>, | ||
| clientAction: ClientAction = 'updated', | ||
| ): Promise<void> => { | ||
| const { _id, ...rest } = data; | ||
| void api.broadcast('watch.priorities', { clientAction, id: _id, diff: { ...rest } }); | ||
| }; |
There was a problem hiding this comment.
// No direct code change, but critical verification is needed.
// Confirm that unconditional broadcasting by this and similar functions is the desired outcome.
// Consider adding a comment to the module or relevant functions explaining why the conditional execution based on `dbWatchersDisabled` was removed, especially if it addresses a specific scenario or architectural decision.This function, along with others in this PR, was previously wrapped by withDbWatcherCheck. The removal of this wrapper means notifications will now always be broadcast. The withDbWatcherCheck HOC executed the notification if dbWatchersDisabled was true (implying DB watchers were off, so manual notification was needed). If dbWatchersDisabled could be false (meaning DB watchers were active and these manual notifications were suppressed), these functions will now broadcast regardless. This could lead to duplicate notifications or unexpected behavior. Please verify that this unconditional notification is the intended behavior across all scenarios.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
|
|
||
| return { oplogEnabled, mongoVersion, mongoStorageEngine, mongo }; | ||
| return { mongoVersion, mongoStorageEngine, mongo }; |
There was a problem hiding this comment.
return { mongoVersion, mongoStorageEngine, mongo };In the fallbackMongoInfo function, the mongoVersion variable is declared (on line 14, an unchanged line) but might remain undefined if the database command in the try block (lines 19-22) fails. When the new return statement on line 38 is executed after such a failure, it would return mongoVersion as undefined. This violates the function's declared return type Promise<{ mongoVersion: string; ... }>, which expects mongoVersion to be a string. This type mismatch could lead to runtime errors or unexpected behavior in consuming code. To ensure type safety, mongoVersion should be initialized to a default string value (e.g., 'unknown') at its declaration, or assigned a default string value within the catch block.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
ggazzo
force-pushed
the
release-8.0.0
branch
2 times, most recently
from
10dc562 to
5bd6ab8
Compare
ggazzo
force-pushed
the
release-8.0.0
branch
4 times, most recently
from
2d38b72 to
191b205
Compare
ggazzo
force-pushed
the
release-8.0.0
branch
3 times, most recently
from
c0306ac to
e71943d
Compare
#36849) Signed-off-by: Abhinav Kumar <[email protected]>
Co-authored-by: Douglas Fabris <[email protected]>
ggazzo
force-pushed
the
release-8.0.0
branch
4 times, most recently
from
b7b6960 to
169b783
Compare
Co-authored-by: Douglas Fabris <[email protected]>
Co-authored-by: Tasso Evangelista <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Guilherme Gazzo [email protected]
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
This pull request, titled "Release 8.0.0," is aimed at merging changes from the
release-8.0.0branch into thedevelopbranch of the RocketChat/Rocket.Chat repository. The key updates in this PR include:Changeset Updates:
@rocket.chat/meteorpackage, specifically noting the removal of the deprecatedcanAccessRoommeteor method.@rocket.chat/meteorpackage, highlighting the removal of support for MongoDB versions 5.x and 6.x.CI Workflow Modifications:
Version and Compatibility Updates:
rocketchat.infohas been updated from 7.7.0-develop to 8.0.0-develop.serverRunning.js, with a startup error enforcing this requirement. The previous system for MongoDB deprecation warnings for versions below 6.0 has been commented out, including its associated logic, imports, and helper variables.