-
Notifications
You must be signed in to change notification settings - Fork 52
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
- F move verify_argument_parser scrubber to caller #185
base: main
Are you sure you want to change the base?
- F move verify_argument_parser scrubber to caller #185
Conversation
Reviewer's Guide by SourceryThe PR moves the responsibility of providing the scrubber function from the verify_argument_parser function to its callers. This simplifies the verify_argument_parser implementation by removing the default scrubber logic and making it the caller's responsibility to handle text scrubbing if needed. Sequence diagram for verify_argument_parser call with scrubbersequenceDiagram
participant Caller
participant verify_argument_parser
participant Options
Caller->>verify_argument_parser: Call with parser and Options().with_scrubber(scrubber)
verify_argument_parser->>Options: Use scrubber from Options
verify_argument_parser->>verify_argument_parser: Format help with parser
verify_argument_parser->>Caller: Return verification result
Updated class diagram for verify_argument_parser functionclassDiagram
class verify_argument_parser {
-parser: ArgumentParser
-options: Optional[Options]
+verify_argument_parser(parser, options)
}
class Options {
+with_scrubber(scrubber)
}
verify_argument_parser --> Options: uses
note for verify_argument_parser "Scrubber logic moved to caller"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
2343a93
to
9e5cdfe
Compare
@JayBazuzi is this still WIP? or can we merge? |
It needs discussion. Llewellyn said he likes things as is. Some people I work with have a very slight preference for this proposed change; I thought it might be a blocking concern from them but they decided it was acceptable. |
9e5cdfe
to
cc0423b
Compare
Not every user of
verify_argument_parser
wants to scrub theoptions
header, or if they want to scrub it they may want to scrub it differently (e.g.[OPTIONS HEADER]
), or they may want to scrub based on the version of Python (if sys.version_info >= (3, 10)...
).