Open
Conversation
|
No correctness or security regressions stood out in the PR’s code changes.
I couldn’t execute the test suite locally in this environment because |
Using library "clickdc", collect CLI arguments into a "cli_args" dataclass. A small amount of reordering was done, which affects the output of the helpdoc, but the intention is for this to introduce no other functional changes. The properties in "cli_args" are sometimes mutated, for example when reading comparable values from a DSN. But mutating the properties is debatable. New tests are introduced for the crucial user/host/port/socket coordinates, but we stop short of adding new tests for every single CLI argument. Motivations * clarity * click_entrypoint() had too many positional arguments * adding an CLI argument required adding both a decorator and a matching positional argument to click_entrypoint() * this work can be a step toward breaking up massive main.py and test_main.py Another related step would be gathering disparate runtime settings into a self.settings property.
b0b5938 to
df2ed72
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Using library
clickdc, collect CLI arguments into acli_argsdataclass.A small amount of reordering was done, which affects the output of the helpdoc, but the intention is for this to introduce no other functional changes.
The properties in
cli_argsare sometimes mutated, for example when reading comparable values from a DSN. But mutating the properties is debatable.New tests are introduced for the crucial user/host/port/socket coordinates, but we stop short of adding new tests for every single CLI argument.
Motivations
click_entrypoint()had too many positional argumentsclick_entrypoint()main.pyandtest_main.pyAnother related step would be gathering disparate runtime settings into a
self.settingsproperty.Checklist
changelog.mdfile.AUTHORSfile (or it's already there).