Closed benthayer closed 4 years ago
@sahansk2 Did I miss anything? How do you like the updates?
@benthayer These updates look good, and make sense!
I just have a question though -- is there a particular reason you defined get_level_identifier()
in GitGud
rather than in Operator
? I'd prefer to have it in Operator
, just because (1) it's convenient if we ever want the functionality of get_level_identifier()
somewhere else, and (2) it doesn't seem to use anything available in __main__.py
that isn't available in operations.py
-- it's only splitting the text and returning the tuple (as opposed to GitGud.get_level()
, for example, which requires all_skills
),
I'm totally cool with the idea of GitGud
having a wrapper function around Operator.get_level_identifier()
, though.
The code was riddled with these when it shouldn't have been the case. This PR fixes that.
Also in this PR, I took the time to refactor and consolidate the code in
handle_load
. We were doing the same thing in multiple spots, so I made sure it was all next to each other and it flowed.