[ECO-5015] Rename public API for {room, connection} lifecycle and status #77
Conversation
We:
- rename the RoomLifecycle enum to RoomLifecycleStatus (which makes more
sense);
- rename the RoomStatus protocol to RoomLifecycle and rename its
`current` property to `status` ("current" what?; it wasn’t clear, and
was inconsistent with the core SDKs, which only use `current` for state
_changes_)
The net effect is that instead of writing `room.status.current` you now
write `room.lifecycle.status`, which still seems pretty readable and
gives the benefits listed above.
This change has been agreed with the Chat team and the decision is
recorded in [1]; they will make this change in JS.
As part of this, I’ve also corrected references to room “state” to the
correct terminology of “status” (in [2] Andy explains why they used the
word “status”, choosing to reserve “state” for some larger composite
state).
Outstanding spec question about whether to rename the RoomInFailedState
error [3]; that can wait.
[1] https://ably.atlassian.net/wiki/spaces/CHA/pages/3307405320/SDK+Divergence+Decision+Log
[2] #73 (comment)
[3] https://github.com/ably/specification/pull/200/files#r1783542331
It was called `current` in 25e5052 to match the naming used in the public API at the time. Post-64c0315, calling it `status` makes more sense.
Same motivation. Resolves #73.
WalkthroughThe pull request introduces significant changes to the Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (13)
Tests/AblyChatTests/DefaultRoomLifecycleTests.swift (3)
11-15: LGTM: Test method correctly verifies initial error state. Consider consistent attribute usage.The
error_startsAsNiltest method effectively checks that a newDefaultRoomLifecycleinstance starts with anilerror. The method name and structure are appropriate.For consistency, consider using the same
@Testattribute format across all test methods. The current method uses@Test()with parentheses, while the previous method used@Testwithout parentheses.
17-40: LGTM: Comprehensive test for status transition. Consider enhancing error handling.The
transitiontest method effectively verifies the behavior of status transition inDefaultRoomLifecycle. It checks for correct emission to subscribers, accurate status changes, and proper updating of the lifecycle's status property. The use of async/await is appropriate for testing asynchronous behavior.Consider enhancing error handling by using a do-catch block around the asynchronous operations. This would allow for more specific error assertions if the asynchronous operations fail. For example:
do { for statusChange in try await [#require(statusChange1), #require(statusChange2)] { #expect(statusChange.previous == originalStatus) #expect(statusChange.current == newStatus) } #expect(await lifecycle.status == .attached) } catch { XCTFail("Unexpected error: \(error)") }This change would provide more context if an error occurs during the asynchronous operations.
1-41: Overall, well-structured and comprehensive tests for DefaultRoomLifecycle.The
DefaultRoomLifecycleTestsprovide a solid set of unit tests for theDefaultRoomLifecycleclass. The tests cover initial state (status and error) and transition behavior, which are crucial aspects of the lifecycle management.Strengths:
- Clear and descriptive test method names
- Appropriate use of async/await for asynchronous testing
- Comprehensive checking of status transition behavior
Areas for potential improvement:
- Ensure consistent usage of the
@Testattribute across all test methods- Consider enhancing error handling in the
transitiontest method- If possible, add more edge cases or error scenarios to increase test coverage
As the codebase grows, consider organizing tests into separate files for different aspects of the
DefaultRoomLifecycleclass if it becomes more complex. This will help maintain test clarity and manageability.Sources/AblyChat/RoomLifecycleManager.swift (2)
82-89: LGTM! Consider enhancing the logging message.The updates to the
changeStatusmethod are consistent with the property renaming and maintain the existing functionality.Consider enhancing the logging message to include more context:
- logger.log(message: "Transitioning from \(status) to \(new), error \(String(describing: error))", level: .info) + logger.log(message: "Room status transitioning from \(status) to \(new), error: \(error?.localizedDescription ?? "none")", level: .info)This change provides clearer context and handles the optional error more elegantly.
Line range hint
1-300: Overall LGTM! Consider addressing TODOs in future work.The changes in this file successfully implement the renaming of the public API for room lifecycle and status. The consistency of the changes throughout the file demonstrates a thorough implementation that maintains the existing functionality while improving naming clarity.
For future work, consider addressing the existing TODOs and known issues, such as:
- Cleaning up old subscriptions (line 72)
- Implementing the "asynchronously with respect to CHA-RL1h4" part of CHA-RL1h5 (line 143)
- Handling task cancellation (line 290)
These improvements would further enhance the robustness and completeness of the
RoomLifecycleManager.Tests/AblyChatTests/RoomLifecycleManagerTests.swift (1)
Line range hint
477-490: Consistent terminology update and a reminder about the TODOThe change from "state change to FAILED" to "status change to FAILED" in the comment maintains the consistency of terminology throughout the test suite. Well done on paying attention to these details.
However, there's a TODO comment on line 479 that references an outstanding question. It might be worth addressing this question or creating a separate issue to track it if it hasn't been done already.
Consider creating a GitHub issue to track the TODO comment if it hasn't been addressed yet.
Sources/AblyChat/Connection.swift (2)
9-9: Address the TODO Regarding Error UnwrappingThe TODO comment suggests considering how to avoid the need for an unwrap of the
errorproperty inConnectionLifecycle.I can assist in proposing a solution to handle
errorsafely, possibly by making use of error-handling patterns or introducing a non-optional error representation. Would you like me to help implement this or open a GitHub issue to track this task?
27-27: Address the TODO Regarding Error Handling in Status ChangesThe TODO comment indicates a need to consider how to avoid unwrapping the
errorproperty inConnectionStatusChange.I can help propose a safer way to handle
error, perhaps by introducing an error wrapper or modifying the type to be non-optional if appropriate. Would you like assistance with this?Sources/AblyChat/RoomLifecycle.swift (5)
3-4: Potential Confusion BetweenRoomLifecycleProtocol andRoomStatusEnumRenaming the protocol to
RoomLifecycleand the enum toRoomStatusmight cause confusion due to the similarity in terms. Consider choosing more distinct names to clearly differentiate between the protocol and the enum, enhancing code readability and maintainability.
4-4: Address the TODO Regarding UnwrappingerrorPropertyThe TODO comment suggests reconsidering the need to unwrap the
errorproperty:// TODO: (https://github.com/ably-labs/ably-chat-swift/issues/12): consider how to avoid the need for an unwrap var error: ARTErrorInfo? { get async }Resolving this could improve error handling within the
RoomLifecycleprotocol.Would you like assistance in proposing a solution to eliminate the need for unwrapping the
errorproperty?
23-24: Update Documentation forRoomStatusChangePropertiesEnsure that the documentation comments for the
currentandpreviousproperties inRoomStatusChangeare updated to reflect the changes fromRoomLifecycletoRoomStatus.
28-28: Consider Enhancing the Initializer's Error HandlingReview if the initializer for
RoomStatusChangeshould provide more clarity or safety regarding theerrorparameter. Enhancing this could improve the robustness of state change representations.
36-36: Populate theerrorProperty as per TODOThe TODO comment indicates that the
errorproperty needs to be populated:// TODO: populate this (https://github.com/ably-labs/ably-chat-swift/issues/28) internal private(set) var error: ARTErrorInfo?Addressing this will enhance error reporting in the
DefaultRoomLifecycleactor.Would you like assistance in implementing the error handling for this property as referenced in issue #28?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
- Sources/AblyChat/Connection.swift (2 hunks)
- Sources/AblyChat/Room.swift (5 hunks)
- Sources/AblyChat/RoomLifecycle.swift (3 hunks)
- Sources/AblyChat/RoomLifecycleManager.swift (9 hunks)
- Tests/AblyChatTests/DefaultRoomLifecycleTests.swift (1 hunks)
- Tests/AblyChatTests/DefaultRoomStatusTests.swift (0 hunks)
- Tests/AblyChatTests/DefaultRoomTests.swift (4 hunks)
- Tests/AblyChatTests/RoomLifecycleManagerTests.swift (25 hunks)
💤 Files with no reviewable changes (1)
- Tests/AblyChatTests/DefaultRoomStatusTests.swift
🧰 Additional context used
📓 Learnings (6)
Sources/AblyChat/Room.swift (1)
Learnt from: lawrence-forooghian PR: ably-labs/ably-chat-swift#67 File: Sources/AblyChat/RoomLifecycleManager.swift:217-220 Timestamp: 2024-10-01T19:44:34.032Z Learning: In `RoomLifecycleManager`, handling of missing `stateChange.reason` when `stateChange.resumed` is `false` is being tracked in issue #74. Future reviews should be aware of this planned work.Sources/AblyChat/RoomLifecycle.swift (1)
Learnt from: lawrence-forooghian PR: ably-labs/ably-chat-swift#67 File: Sources/AblyChat/RoomLifecycleManager.swift:217-220 Timestamp: 2024-10-01T19:44:34.032Z Learning: In `RoomLifecycleManager`, handling of missing `stateChange.reason` when `stateChange.resumed` is `false` is being tracked in issue #74. Future reviews should be aware of this planned work.Sources/AblyChat/RoomLifecycleManager.swift (1)
Learnt from: lawrence-forooghian PR: ably-labs/ably-chat-swift#67 File: Sources/AblyChat/RoomLifecycleManager.swift:217-220 Timestamp: 2024-10-01T19:44:34.032Z Learning: In `RoomLifecycleManager`, handling of missing `stateChange.reason` when `stateChange.resumed` is `false` is being tracked in issue #74. Future reviews should be aware of this planned work.Tests/AblyChatTests/DefaultRoomLifecycleTests.swift (1)
Learnt from: lawrence-forooghian PR: ably-labs/ably-chat-swift#54 File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:335-335 Timestamp: 2024-09-23T19:56:39.435Z Learning: In Swift Testing, test method names do not need to start with `test`.Tests/AblyChatTests/DefaultRoomTests.swift (3)
Learnt from: lawrence-forooghian PR: ably-labs/ably-chat-swift#54 File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:627-631 Timestamp: 2024-10-01T12:55:21.968Z Learning: The closure `detachImpl` in `RoomLifecycleManagerTests.swift` functions as intended without external state variables to track invocation count.Learnt from: lawrence-forooghian PR: ably-labs/ably-chat-swift#54 File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:335-335 Timestamp: 2024-09-23T19:56:39.435Z Learning: In Swift Testing, test method names do not need to start with `test`.Learnt from: lawrence-forooghian PR: ably-labs/ably-chat-swift#67 File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:941-941 Timestamp: 2024-10-01T19:46:16.467Z Learning: When a TODO comment references an existing GitHub issue, it's acceptable to leave it in the code without requesting its removal.Tests/AblyChatTests/RoomLifecycleManagerTests.swift (5)
Learnt from: lawrence-forooghian PR: ably-labs/ably-chat-swift#54 File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:335-335 Timestamp: 2024-09-23T19:56:39.435Z Learning: In Swift Testing, test method names do not need to start with `test`.Learnt from: lawrence-forooghian PR: ably-labs/ably-chat-swift#67 File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:941-941 Timestamp: 2024-10-01T19:46:16.467Z Learning: When a TODO comment references an existing GitHub issue, it's acceptable to leave it in the code without requesting its removal.Learnt from: lawrence-forooghian PR: ably-labs/ably-chat-swift#67 File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:836-836 Timestamp: 2024-10-01T19:46:04.461Z Learning: Before suggesting the removal of TODO comments, check if they reference existing GitHub issues that track the pending work.Learnt from: lawrence-forooghian PR: ably-labs/ably-chat-swift#54 File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:627-631 Timestamp: 2024-10-01T12:55:21.968Z Learning: The closure `detachImpl` in `RoomLifecycleManagerTests.swift` functions as intended without external state variables to track invocation count.Learnt from: lawrence-forooghian PR: ably-labs/ably-chat-swift#54 File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:499-504 Timestamp: 2024-10-01T12:47:28.179Z Learning: In test closures, parameters like `callCount` may be used to track invocation counts correctly across invocations.
🔇 Additional comments (39)
Tests/AblyChatTests/DefaultRoomLifecycleTests.swift (3)
1-2: LGTM: Import statements are appropriate.The import statements correctly import the necessary modules for testing. The
@testableattribute onAblyChatimport allows access to internal members for testing purposes.
4-4: LGTM: Struct declaration is clear and purposeful.The
DefaultRoomLifecycleTestsstruct name clearly indicates its purpose to test theDefaultRoomLifecycleclass. The structure aligns with the custom testing framework being used.
5-9: LGTM: Test method correctly verifies initial status.The
status_startsAsInitializedtest method effectively checks that a newDefaultRoomLifecycleinstance starts with the.initializedstatus. The method name and structure align with the custom testing framework and previous learnings.Tests/AblyChatTests/DefaultRoomTests.swift (5)
22-22: LGTM: Consistent with PR objectivesThe change from
room.statustoroom.lifecyclealigns with the PR's goal of renaming the public API for room lifecycle and status. This modification enhances clarity while maintaining the existing behavior with thebufferingPolicy: .unboundedparameter.
84-84: LGTM: Consistent with previous changesThe modification from
room.statustoroom.lifecycleis consistent with the changes made in theattachfunction. This change maintains the existing behavior while aligning with the PR's objective to rename the public API for room lifecycle and status.
Line range hint
1-138: Summary: Consistent implementation of API changesThe changes in this file consistently implement the renaming of
room.statustoroom.lifecycleacross all relevant test methods. This aligns well with the PR objectives to rename the public API for room lifecycle and status. The modifications maintain the existing test logic and structure while updating the API usage.Key points:
- All occurrences of
room.statushave been changed toroom.lifecycle.- The
currentproperty is no longer used, now directly accessingstatusfromlifecycle.- Test logic and assertions remain unchanged, focusing on the correct states (
attachedordetached).These changes contribute to improving the clarity and consistency of the public API as intended by the pull request.
95-95: LGTM: Consistent change, verify across all test casesThe change from
room.status.currenttoroom.lifecycle.statusis consistent with previous modifications in both theattachanddetachfunctions. This maintains the consistency of the new API structure throughout the test file.To ensure consistency across all test cases in this file, please run the following script:
#!/bin/bash # Description: Verify consistent usage of room.lifecycle.status in all test cases # Test: Search for any remaining occurrences of room.status in the current file rg --type swift 'room\.status' Tests/AblyChatTests/DefaultRoomTests.swift # Test: Confirm all occurrences now use room.lifecycle.status rg --type swift 'room\.lifecycle\.status' Tests/AblyChatTests/DefaultRoomTests.swift
33-33: LGTM: Consistent change, verify API structureThe change from
room.status.currenttoroom.lifecycle.statusis consistent with the previous modification and aligns with the PR objectives. The removal of thecurrentproperty suggests a change in the API structure.To ensure the new API structure is used consistently throughout the codebase, please run the following script:
✅ Verification successful
LGTM: API structure verified successfully
All instances of
room.status.currenthave been removed and successfully replaced withroom.lifecycle.status. The changes are consistent across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of room.lifecycle.status across the codebase # Test: Search for occurrences of room.lifecycle.status rg --type swift 'room\.lifecycle\.status' # Test: Search for any remaining occurrences of room.status.current rg --type swift 'room\.status\.current'Length of output: 278
Sources/AblyChat/RoomLifecycleManager.swift (5)
38-38: LGTM! Consistent renaming in initialization methods.The renaming of the
currentparameter tostatusin all initialization methods is consistent with the property renaming and maintains the existing functionality while improving naming consistency.Also applies to: 47-47, 53-53, 62-62
100-100: LGTM! Consistent update inperformAttachOperation.The switch statement in
performAttachOperationhas been correctly updated to usestatusinstead ofcurrent, maintaining consistency with the property renaming while preserving the method's functionality.
174-174: LGTM! Consistent updates inperformDetachOperation.The changes in the
performDetachOperationmethod, including the switch statement and the condition in the if statement, have been correctly updated to usestatusinstead ofcurrent. These updates maintain consistency with the property renaming while preserving the method's functionality.Also applies to: 220-222
253-253: LGTM! Consistent update inperformReleaseOperation.The switch statement in
performReleaseOperationhas been correctly updated to usestatusinstead ofcurrent, maintaining consistency with the property renaming while preserving the method's functionality.
25-25: LGTM! Verify theRoomStatustype definition.The renaming of
currenttostatusimproves clarity and aligns with common naming conventions for state properties. This change is consistent with the PR objectives.Please ensure that the
RoomStatustype is properly defined and includes all necessary states from the previousRoomLifecycleenum. Run the following script to verify:✅ Verification successful
Verified:
RoomStatustype is correctly defined with all necessary states.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the RoomStatus type definition # Test: Search for the RoomStatus type definition rg --type swift -A 10 'enum RoomStatus'Length of output: 652
Tests/AblyChatTests/RoomLifecycleManagerTests.swift (17)
31-39: Great job on consistent renaming!The changes from
currenttostatusare applied consistently here. The use of thetestsOnly_prefix for the initialization parameter is a good practice, clearly indicating that it's intended for testing purposes only.
64-67: Excellent update to test method and assertion!The test method name and assertion have been correctly updated to reflect the change from
currenttostatus. This maintains consistency and clarity in the test suite.
82-85: Good attention to detail in comment updates!The change from "ATTACHED state" to "ATTACHED status" in the comment is a nice touch. It maintains consistency with the new terminology used throughout the codebase.
96-99: Consistent terminology in comments!The update from "RELEASING state" to "RELEASING status" in the comment maintains the consistency of terminology throughout the test suite. Well done on paying attention to these details.
111-114: Maintaining consistency in comments!The change from "RELEASED state" to "RELEASED status" in the comment is another good example of maintaining consistent terminology throughout the test suite.
Line range hint
126-141: Thorough update of test case!Excellent job on updating all relevant parts of this test case. The comments and assertions now consistently use "status" instead of "current", maintaining the new terminology throughout the test.
159-165: Consistent updates in comments and assertions!The changes in this segment maintain consistency with the new terminology. Both the comment and the assertion have been updated to use "status" instead of "current" or "state". This attention to detail helps maintain a clear and consistent codebase.
193-198: Excellent consistency in terminology!The updates in this segment demonstrate a thorough approach to the renaming effort. Both the comment and the assertion have been correctly updated to use "status" instead of "state" or "current". This consistency helps maintain clarity throughout the test suite.
243-248: Consistent terminology in comments and assertions!The changes in this segment continue to demonstrate a thorough approach to the renaming effort. Both the comment and the assertion have been correctly updated to use "status" terminology, maintaining consistency throughout the test suite.
Line range hint
400-414: Thorough update of test case!Excellent work on updating all relevant parts of this test case. The comments and assertions now consistently use "status" instead of "current", maintaining the new terminology throughout the test. This attention to detail ensures clarity and consistency in the test suite.
432-438: Consistent updates in comments and assertions!The changes in this segment maintain consistency with the new terminology. Both the comment and the assertion have been updated to use "status" instead of "current" or "state". This attention to detail helps maintain a clear and consistent codebase throughout the test suite.
Line range hint
530-538: Maintaining consistency in comments!The change from "RELEASED state" to "RELEASED status" in the comment is another good example of maintaining consistent terminology throughout the test suite. This attention to detail contributes to a more coherent and understandable codebase.
Line range hint
544-556: Consistent updates in comments and assertions!The changes in this segment maintain consistency with the new terminology. Both the comment and the assertion have been updated to use "status" instead of "current" or "state". This attention to detail helps maintain a clear and consistent codebase throughout the test suite.
Line range hint
563-577: Thorough update of test case!Excellent work on updating all relevant parts of this test case. The comments and assertions now consistently use "status" instead of "current", maintaining the new terminology throughout the test. This attention to detail ensures clarity and consistency in the test suite.
617-617: Consistent update in assertion!The change in this assertion maintains consistency with the new terminology, using
manager.statusinstead ofmanager.current. This small but important update contributes to the overall consistency of the test suite.
676-676: Consistent update in final assertion!The change in this final assertion maintains consistency with the new terminology, using
manager.statusinstead ofmanager.current. This update ensures that the entire test file consistently uses the new "status" terminology.
Line range hint
1-679: Excellent job on consistent renaming throughout the file!The changes in this file demonstrate a thorough and consistent effort to rename
currenttostatus. All relevant comments, method names, and assertions have been updated to reflect this change. This renaming improves the clarity and consistency of the codebase without altering its functionality.A few key points:
- The use of
testsOnly_prefix for test-specific parameters is a good practice.- Consistency in terminology between comments and code has been maintained.
- All assertions have been correctly updated to use
manager.status.The only minor suggestion would be to address the TODO comment on line 479 by creating a GitHub issue if it hasn't been done already.
Overall, this is a well-executed refactoring that enhances the readability and maintainability of the test suite.
Sources/AblyChat/Connection.swift (5)
4-4: Renaming 'status' to 'lifecycle' Enhances ClarityThe change from
statustolifecyclein theConnectionprotocol improves semantic clarity by encapsulating the entire lifecycle of the connection rather than just its current status.
7-8: Introduction of 'ConnectionLifecycle' Protocol with 'status' PropertyDefining a new
ConnectionLifecycleprotocol that includes astatusproperty of typeConnectionStatusprovides a clean abstraction for the connection's lifecycle management. This allows for future extensions and better organization of lifecycle-related properties and methods.
Line range hint
14-19: Renaming Enum to 'ConnectionStatus' Improves ReadabilityRenaming the
ConnectionLifecycleenum toConnectionStatusaccurately reflects its purpose of representing the connection's status. This enhances code readability and aligns with common naming conventions.
24-25: Updating 'ConnectionStatusChange' Properties for ConsistencyChanging the
currentandpreviousproperties to useConnectionStatusmaintains consistency with the renamed enum and ensures that the status changes are accurately tracked.
Line range hint
30-34: Initializer Update Aligns with Property ChangesThe initializer for
ConnectionStatusChangenow accepts parameters of typeConnectionStatus, aligning with the updated properties. This ensures consistency and simplifies the initialization process.Sources/AblyChat/RoomLifecycle.swift (1)
55-59: Verify State Transition Logic intransitionMethodThe
transitionmethod updatesstatusand notifies subscribers of status changes:internal func transition(to newStatus: RoomStatus) { logger.log(message: "Transitioning to \(newStatus)", level: .debug) let statusChange = RoomStatusChange(current: newStatus, previous: status) status = newStatus for subscription in subscriptions { subscription.emit(statusChange) } }Ensure that all possible state transitions are valid and handled correctly. Consider edge cases where additional logic might be necessary to prevent invalid transitions or to handle specific scenarios.
Sources/AblyChat/Room.swift (3)
33-33: Initialization of_lifecyclelooks correctDeclaring
_lifecycleas an instance ofDefaultRoomLifecyclesets up the room's lifecycle management properly.
41-41: Proper initialization of_lifecyclein the initializerInitializing
_lifecyclewith the providedloggerensures that lifecycle events are logged appropriately.
64-65: Nonisolated propertylifecyclecorrectly exposes_lifecycleThe computed property
lifecyclecorrectly provides access to_lifecyclewhile respecting actor isolation requirements.
|
Closing; will open new PR. |
Resolves #73, assuming that we want to make the same changes for connection as we do for room (asked in #73 (comment)). See commit messages for details.
Update: Seems that this is not fully decided; conversation ongoing on this DR now. Will move back to draft.
Summary by CodeRabbit
New Features
DefaultRoomLifecycleclass to validate its behavior and state transitions.Bug Fixes
Documentation