Refactor main.py to minimal content#1894
Conversation
rolandwalker
force-pushed
the
RW/minimal-main-py-refactor-20260518
branch
from
72f7c88 to
5819fb6
Compare
| echo_calls: list[str] = [] | ||
| cli.echo = lambda message, **kwargs: echo_calls.append(str(message)) # type: ignore[assignment] | ||
| monkeypatch.setattr(main, 'WIN', False) | ||
| monkeypatch.setattr(mycli.compat, 'WIN', False) |
There was a problem hiding this comment.
This should be: monkeypatch.setattr(mycli.client_connection, 'WIN', True) I believe, like in test_main_regression.py
| logger = DummyLogger() | ||
| cli.logger = cast(Any, logger) | ||
| monkeypatch.setattr(main, 'WIN', True) | ||
| monkeypatch.setattr(mycli.client_connection, 'WIN', True) |
There was a problem hiding this comment.
This is the correct one I believe; all the rest should match (this file, and test_main.py)
| click.secho("No database selected", err=True, fg="red") | ||
| return | ||
|
|
||
| from mycli import main as main_module |
There was a problem hiding this comment.
Dupe import of below with comment
| except IOError as e: | ||
| return [SQLResult(status=str(e))] | ||
|
|
||
| from mycli import main as main_module |
|
|
||
| if cli_args.checkup: | ||
| main_module.main_checkup(mycli) | ||
| main_module.sys.exit(0) |
There was a problem hiding this comment.
For all of these sys commands, is there a reason you're accessing that through the module instead of just importing sys at the top and using that as normal?
There was a problem hiding this comment.
No good reason! I (and the agent) kept the tests working against main, which was awkward. This and other weird things got through due to that constraint/pattern.
The goal is to not pull anything from main; the question is whether to keep this PR open while all of that incremental work is done.
rolandwalker
force-pushed
the
RW/minimal-main-py-refactor-20260518
branch
from
5819fb6 to
22eef14
Compare
rolandwalker
changed the title
main.py to minimal contentmain.py to minimal content
creates the following: * mycli/cli_runner.py * mycli/client.py * mycli/client_commands.py * mycli/client_connection.py * mycli/client_query.py and reduces main.py to only: * click_entrypoint() * main()
rolandwalker
force-pushed
the
RW/minimal-main-py-refactor-20260518
branch
from
22eef14 to
14a53c7
Compare
|
Even though there are some obvious followups wanted, I've convinced myself to merge this as-is (after accounting for review comments). It may be valuable to have the steps of refactoring in the commit history. |
2 participants
Description
Creates the following:
mycli/cli_runner.pymycli/client.pymycli/client_commands.pymycli/client_connection.pymycli/client_query.pyand reduces
main.pyto only:click_entrypoint()main()As first PR'd, this maintains the current testing surface. Best would be to reach the goal of individual tests over each of the above files, and outright deletion of
test_main_regression.py. Edit: keeping the structure as-is for this PR, and looking to followups to resolve.Also, as first PR'd,
main.pycontains almost nothing. It would be logical to move the content ofcli_args.pyback in. Edit: keeping the structure as-is for this PR, and looking to followups to resolve.While the files are smaller and the code is more approachable, like previous refactors, this is not perfect. For example,
cli_runner.pyhas extensive logic regarding environment variables and command-line arguments. Whereas, ideally, all configuration would be collected together in a distinct layer.Checklist
changelog.mdfile.AUTHORSfile (or it's already there).