Make some data class immutable #5762
Conversation
| ?.let { existingPollSummaryContent -> | ||
| eventAnnotationsSummaryEntity.pollResponseSummary?.aggregatedContent = ContentMapper.map( | ||
| PollSummaryContent( | ||
| myVote = existingPollSummaryContent.myVote, |
There was a problem hiding this comment.
This code is quite weird to me the goal is to only keep myVote? CC @onurays
There was a problem hiding this comment.
Yes, it is to help reach myVote easily while rendering it.
There was a problem hiding this comment.
not for this PR, might be better suited as a separate type if possible 🤔
There was a problem hiding this comment.
Can you clarify? You mean having something like data class MutablePollSummaryContent with var and mutable types and a toPollSummaryContent() method, or is it something else?
There was a problem hiding this comment.
I meant a dedicated MyVotePollContent(myVote), avoiding the need for the other values to be set to default values
| @@ -50,6 +50,7 @@ ext.groups = [ | |||
| 'com.beust', | |||
| 'com.davemorrissey.labs', | |||
| 'com.dropbox.core', | |||
| 'com.soywiz.korlibs.korte', | |||
There was a problem hiding this comment.
was this missing from the previous PR? 🤔 thought the CI would catch those errors
There was a problem hiding this comment.
The CI does not call (yet) ./gradlew dokkaHtml which support has been added by the previous PR, and this addition is to be able to call ./gradlew dokkaJavadoc (as per the commit message)
| /** | ||
| * Flag to indicate the signature from this device is valid. | ||
| */ | ||
| val valid: Boolean, |
There was a problem hiding this comment.
Should we rename it as isValid?
There was a problem hiding this comment.
Yes, you are right, something for a future PR :)
Following #5726 where some TODOs has been added, some data class was using
varinstead ofval. This PR fixes that.Hopefully it will not break anything, but please review carefully.