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
E1771. [TLD] Refactor team.rb #1043
Conversation
Renaming get_node_type to node_type
Renaming get_author_names to author_names
Passing &:destroy to the each block for destroying the team
Using find_by instead of dynamic methods and where.first
Updating the tests in team_spec.rb
Dividing randomize_all_by_parent method into multiple sub methods Change done by sjbarai
0ced4de
to
01b3c75
Compare
a79173d
to
d72e055
Compare
Hi team, You refactoring looks good to me.
Thanks, |
Hi Zhewei, Thanks for the update. We'll complete the tests and try to remove the code climate errors and update the pull request. Thanks, |
Completed the remaining tests
Removing some minor and straight forward codeclimate errors
Solved a few more codeclimate issues
Reverting to use of each instead of find_each
end | ||
end | ||
|
||
def self.sort_teams_by_members_reverse(teams) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, I don't think you need to extract one line of code as a new method. But it is not a big deal.
These are all the minor refactoring related to the file. The two tasks left are updating the tests and refactoring self.randomize_all_by_parent function by disintegrating it in other smaller function. We are planning to submit these in next 3 days (by updating this pull request and removing the WIP mark).