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

Reduce boilerplate with some minor sdk improvements #6799

Merged
merged 5 commits into from Aug 12, 2022

Conversation

Florian14
Copy link
Contributor

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Improve some sdk stuff:

  • Provide JSON keys inside enum fields as a value for GuestAccess and RoomHistoryVisibility (It is already the case for some existing enums) -> It limits the duplication of the keys by having them in a single place
  • Replace manual mapping with the related Content models in CreateRoomBodyBuilder -> it removes some JSON keys duplication and adds clarity to the code
  • Replace the return type of RoomFeaturePreset.setupInitialStates from a List<Event> to a List<CreateRoomStateEvent> to be consistent with CreateRoomParams.initialStates

Motivation and context

Further developments related to #5525

Tests

By creating rooms in order to trigger the modified code, adding breakpoints and checking that the code was correctly executed.

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@@ -0,0 +1 @@
[Create Room] Reduce some boilerplate with room state event contents
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about the changelog description, maybe I should have split the PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see anything wrong. It makes perfect sense to me

@Florian14 Florian14 requested review from a team and ericdecanini and removed request for a team August 10, 2022 08:47
@Florian14 Florian14 added the PR-Small PR with less than 20 updated lines label Aug 10, 2022
@Florian14 Florian14 force-pushed the misc/fre/minor_sdk_improvements branch 2 times, most recently from 4923b95 to bf8a8ae Compare August 11, 2022 14:35
Copy link
Contributor

@ericdecanini ericdecanini left a comment

Choose a reason for hiding this comment

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

Nice cleanup! It can also be further simplified by adding a companion object function in RoomHistoryVisibility

    companion object {
        fun where(value: String) = values().find { it.value == value }
    }

Consider it optional

@Florian14
Copy link
Contributor Author

Nice cleanup! It can also be further simplified by adding a companion object function in RoomHistoryVisibility

    companion object {
        fun where(value: String) = values().find { it.value == value }
    }

Consider it optional

I like your proposal, I will probably do it in a new PR in a more general way!

@Florian14 Florian14 force-pushed the misc/fre/minor_sdk_improvements branch from bf8a8ae to a3ecb43 Compare August 12, 2022 08:32
@sonarcloud
Copy link

sonarcloud bot commented Aug 12, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

81.6% 81.6% Coverage
0.0% 0.0% Duplication

@Florian14 Florian14 merged commit 62f7b40 into develop Aug 12, 2022
@Florian14 Florian14 deleted the misc/fre/minor_sdk_improvements branch August 12, 2022 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Small PR with less than 20 updated lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants