Refactor FXIOS-10205 [Swiftlint] Resolve 1 implicitly_unwrapped_optional violations in BottomSheetViewController #23795
Conversation
tonell-m
changed the title
| let contentViewBottomConstraint = sheetView.bottomAnchor.constraint(equalTo: view.bottomAnchor) | ||
| self.contentViewBottomConstraint = contentViewBottomConstraint |
There was a problem hiding this comment.
Is this change needed as part of the unwrapped optional change? Couldn't we continue to set the classes property directly without needing to create an intermediate property?
There was a problem hiding this comment.
Hi 👋
This property is then used in NSLayoutContraints.activate([...]), so there is two possible approaches here, either use an intermediate property like this that will be non optional, or assign the property directly and right after add a guard let statement to safely unwrap the property before using it in the constraints array, which seemed a bit awkward to me for a property that is just initialized here.
What do you think?
There was a problem hiding this comment.
Ah I see. That makes sense!
There was a problem hiding this comment.
Hi @Cramsden, as suggested by @PARAIPAN9 in #23833 it's actually more consistent with the rest of the codebase to just activate the newly created constraint in place after initialising it as a class property instead of using a temporary local property, see the latest commit for details.
lmarceau
changed the title
ComponentLibrary: Coverage: 32.33
Generated by 🚫 Danger Swift against ad250b0 |
…nal violations in BottomSheetViewController (#23795) * Resolve warnings in BottomSheetViewController * Initialise and activate constraint in place --------- Co-authored-by: Marceau Tonelli <[email protected]>
📜 Tickets
Jira ticket
Github issue
💡 Description
Continuing resolving
implicitly_unwrapped_optionalviolations in order to enable the rule for further developments.📝 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120)