Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deprecated restricted lab option #4889

Merged
merged 6 commits into from Jan 11, 2022

Conversation

BillCarsonFr
Copy link
Member

Removes the deprecated use experimental restricted join rule lab option

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not touch the translated strings, I do not want to have conflict with Weblate.
See https://github.com/vector-im/element-android/blob/main/CONTRIBUTING.md#removing-existing-strings for more details

@github-actions
Copy link

github-actions bot commented Jan 10, 2022

Unit Test Results

  66 files  ±0    66 suites  ±0   57s ⏱️ -2s
135 tests ±0  135 ✔️ ±0  0 💤 ±0  0 ±0 
418 runs  ±0  418 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit da94bfb. ± Comparison against base commit 56ff5f7.

♻️ This comment has been updated with latest results.

@BillCarsonFr
Copy link
Member Author

Please do not touch the translated strings, I do not want to have conflict with Weblate. See https://github.com/vector-im/element-android/blob/main/CONTRIBUTING.md#removing-existing-strings for more details

Updated

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my remark also the CI is not happy:
w: /workdir/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomViewModel.kt: (58, 55): Parameter 'vectorPreferences' is never used

@@ -72,7 +72,7 @@ class CreateRoomViewModel @AssistedInject constructor(@Assisted private val init
val restrictedSupport = session.getHomeServerCapabilities().isFeatureSupported(HomeServerCapabilities.ROOM_CAP_RESTRICTED)
val createRestricted = when (restrictedSupport) {
HomeServerCapabilities.RoomCapabilitySupport.SUPPORTED -> true
HomeServerCapabilities.RoomCapabilitySupport.SUPPORTED_UNSTABLE -> vectorPreferences.labsUseExperimentalRestricted()
// HomeServerCapabilities.RoomCapabilitySupport.SUPPORTED_UNSTABLE -> vectorPreferences.labsUseExperimentalRestricted()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to just remove instead of commenting all the lines like this one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I hesitated a bit but will remove now (done)

@@ -75,7 +75,7 @@ class RoomSettingsViewModel @AssistedInject constructor(@Assisted initialState:
val restrictedSupport = homeServerCapabilities.isFeatureSupported(HomeServerCapabilities.ROOM_CAP_RESTRICTED)
val couldUpgradeToRestricted = when (restrictedSupport) {
HomeServerCapabilities.RoomCapabilitySupport.SUPPORTED -> true
HomeServerCapabilities.RoomCapabilitySupport.SUPPORTED_UNSTABLE -> vectorPreferences.labsUseExperimentalRestricted()
// HomeServerCapabilities.RoomCapabilitySupport.SUPPORTED_UNSTABLE -> vectorPreferences.labsUseExperimentalRestricted()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still a change to do here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx, updated (+ rebase)

@BillCarsonFr BillCarsonFr force-pushed the feature/bca/clean_deprecated_lab_restricted branch from f88cf06 to da94bfb Compare January 11, 2022 08:34
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update. I will squash and merge.

@bmarty bmarty enabled auto-merge (squash) January 11, 2022 08:47
@bmarty bmarty merged commit bf447af into develop Jan 11, 2022
@bmarty bmarty deleted the feature/bca/clean_deprecated_lab_restricted branch January 11, 2022 09:01
@@ -0,0 +1 @@
Remove deprecated experimental restricted space lab option
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: using removal extension is reserved for changes in the SDK. For this change misc would have been better. (I am seeing that when doing the release, so I will update the changelog manaully, no worries)

onurays pushed a commit that referenced this pull request Jan 11, 2022
* develop: (281 commits)
  Add a comment about the workaround
  Remove deprecated restricted lab option (#4889)
  Bump actions/github-script from 3 to 5.1.0
  Add some missing language in the change language screen
  Workaround to not to reuse poll option cells.
  Olm lib is now hosted in MavenCentral. Upgrade to 3.2.10
  Changelog
  Test: Fix test after change on OnBoarding screens
  Test: Analytics opt-in
  Cleanup
  Revert "Disable automatic opt-in screen display."
  - Do not add GitHub comments on successful ktlint runs  - Remove already existing comments when ktlint succeed
  restore deprecated lab preference
  Update nb of enum classes
  Change autoUisi label + rename matching_issue
  code review
  Fix enabling was broken
  Simple rate limiting of RS sending
  use flow instead of reactivex
  Fix UISIS preference listener
  ...

# Conflicts:
#	vector/src/main/java/im/vector/app/features/attachments/AttachmentTypeSelectorView.kt
#	vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailFragment.kt
#	vector/src/main/res/layout/view_attachment_type_selector.xml
#	vector/src/main/res/values/strings.xml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants