[fact] Refactor the dispatch module #976
Conversation
|
@algomaster99 Ping for review :) I've also added you to the @repobee/reviewers team so you can do PR triage. |
src/_repobee/cli/dispatch.py
Outdated
| def _dispatch_core_command( | ||
| args: argparse.Namespace, config: plug.Config, api: plug.PlatformAPI | ||
| ) -> Mapping[str, List[plug.Result]]: | ||
| dispatch_table = { | ||
| plug.cli.CoreCommand.repos: _dispatch_repos_command, | ||
| plug.cli.CoreCommand.issues: _dispatch_issues_command, | ||
| plug.cli.CoreCommand.config: _dispatch_config_command, | ||
| plug.cli.CoreCommand.reviews: _dispatch_reviews_command, | ||
| plug.cli.CoreCommand.teams: _dispatch_teams_command, | ||
| } | ||
| return dispatch_table[args.category](args, config, api) or {} | ||
|
|
||
| is_ext_command = "_extension_command" in args | ||
| if is_ext_command: | ||
| ext_cmd = args._extension_command | ||
| res = ( | ||
| ext_cmd.command(api=api) | ||
| if ext_cmd.__requires_api__() | ||
| else ext_cmd.command() | ||
| ) | ||
| hook_results = ( | ||
| {str(ext_cmd.__settings__.action): [res]} if res else hook_results | ||
| ) | ||
| else: | ||
| category = args.category | ||
| hook_results = ( | ||
| dispatch_table[category](args, config, api) or hook_results | ||
| ) | ||
|
|
||
| if is_ext_command or args.action in [ | ||
| plug.cli.CoreCommand.repos.setup, | ||
| plug.cli.CoreCommand.repos.update, | ||
| plug.cli.CoreCommand.repos.clone, | ||
| ]: | ||
| if hook_results and any(hook_results.values()): | ||
| plug.echo(formatters.format_hook_results_output(hook_results)) | ||
| if hook_results and "hook_results_file" in args and args.hook_results_file: | ||
| _handle_hook_results( | ||
| hook_results=hook_results, filepath=args.hook_results_file | ||
| ) | ||
| def _dispatch_extension_command( | ||
| ext_cmd: plug.cli.Command, api: plug.PlatformAPI | ||
| ) -> Mapping[str, List[plug.Result]]: | ||
| res = ( | ||
| ext_cmd.command(api=api) | ||
| if ext_cmd.__requires_api__() | ||
| else ext_cmd.command() | ||
| ) | ||
|
|
||
| return hook_results | ||
| return {str(ext_cmd.__settings__.action): [res]} if res else {} |
There was a problem hiding this comment.
You can swap their order. Implement _dispatch_extension_command first because it comes first in the code while reading from top to down.
There was a problem hiding this comment.
This is a valid point, and I did consider it for a while. However, I decided to keep the current order, because I want to emphasize the core dispatch being the "normal" dispatch (i.e. putting it first). At the same time, I don't want to reverse the logic where the selection is made (i.e. dispatching to either the core command dispatcher or extension command dispatcher depending on is_extension_command) because that requires a not, and frankly, negations are hard!
When I was thinking this through, i did however spot that I had a pointless abbreviation: ext_cmd instead of extension_command. I have a bad habit of pointless abbreviations. Just write the full words, it's almost always more readable :)
There was a problem hiding this comment.
that requires a not, and frankly, negations are hard!
Lesser the boolean variables/operators, better is the readability. :)
When I was thinking this through, i did however spot that I had a pointless abbreviation: ext_cmd instead of extension_command.
I agree. It is only beneficial to abbreviate if super long names. By super long I mean those variables whose single usage exceeds the line length.
| def _handle_hook_results( | ||
| args: argparse.Namespace, hook_results: Mapping | ||
| ) -> None: | ||
| if _should_echo_hook_results(args, hook_results): |
There was a problem hiding this comment.
Just a note here - should_print_hook_results_to_STDOUT is a better name because echo is specific to mainly Unix-style operating systems. However, repobee doesn't support Windows yet so it's cool.
There was a problem hiding this comment.
It refers to the function in the plugin system that's called, plug.echo. It prints to stdout and to the log file. So it's a bit more specific than printing to stdout.
Pure refactoring of the
dispatchmodule to improve readability.