Refactor FXIOS-7301 [Swiftlint] Remove 1 closure_body_length violation from MainMenuState.swift and decrease threshold #23991
Conversation
| accountData: state.accountData, | ||
| accountIcon: state.accountIcon | ||
| ) | ||
| return handleViewDidLoadAction(state: state) |
There was a problem hiding this comment.
It looks good to me, only thing i see that basically handleViewDidLoadAction and handleUpdateAccountHeaderAction implement the same state as default state.
@adudenamedruby what do you think if we default those 2 action directly in the default case ? so we can delete this two action and the related handle methods.
There was a problem hiding this comment.
handleUpdateAccountHeaderAction looks different because it's passing in the action, so it's not the same as handleViewDidLoadAction unless I'm missing something?
There was a problem hiding this comment.
Ah true sorry Rou i didn't see the action, yes then they are different. Then i guess we could leave it as is, so in case view did load action changes in the future, we already have the handler
There was a problem hiding this comment.
I'll review this soon. I need to talk to some colleagues about thoughts on this, as, given this is a Redux pattern thing, we should try and be consistent, overall, about how we structure some of this stuff. There's nothing wrong with the PR, just a team discussion.
Client.app: Coverage: 32.4
Generated by 🚫 Danger Swift against 021ae93 |
There was a problem hiding this comment.
Heya @ionixjunior. So we've talked, and decided that we're good to go with the approach in this PR. There's just some minor changes needed and then we can push it through. THanks for your patience, and your hard work!!!
|
@FilippoZazzeroni I'm going on PTO, if you wouldn't mind taking this over the finish line when @ionixjunior pushes their fixes |
ionixjunior
force-pushed
the
issue_16442_part_XXXVIX
branch
from
d974233 to
021ae93
Compare
FilippoZazzeroni
requested review from
adudenamedruby
and removed request for
adudenamedruby
FilippoZazzeroni
requested review from
adudenamedruby
and removed request for
adudenamedruby
|
@ionixjunior i'm trying to find a way to merge this PR, since i see @adudenamedruby still needs to approve since it asked for changes in first place |
|
@FilippoZazzeroni no worries at all, and thank you for letting me know. |
|
;) |
📜 Tickets
Jira ticket
Github issue
💡 Description
This PR removes 1
closure_body_lengthviolation from theMainMenuState.swiftfile. Also, this PR decrease the warning and error threshold to 68.This PR is part of a series of PRs described here #16442 (comment) and here #16442 (comment).
📝 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120)