Skip to content

Conversation

@studyingegret
Copy link

@studyingegret studyingegret commented Aug 9, 2025

This PR doesn't benefit users except that more information (argparse description) is displayed in subcommands' help (e.g. dmypy start -h).

Formats argparse code of dmypy like this

    p = add_subparser("start", action=start_action,
                      help="Start daemon (exits with error if already running)")
    p.add_argument("--log-file", metavar="FILE", type=str,
                   help="Direct daemon stdout/stderr to FILE")
    p.add_argument("--timeout", metavar="TIMEOUT", type=int,
                   help="Server shutdown timeout (in seconds)")

It feels maybe redundant to explain the style, but still:

  • The option options are placed in the first line.

    ("--log-file", metavar="FILE", type=str,

  • The help has its own line.

    help="Direct daemon stdout/stderr to FILE")

  • With these two parts uniformly, visually separated and horizonally aligned, it is easier to understand the options by reading the option options and help text.

Also mypy/dmypy_(os,server,util).py are moved to dmypy directory because before it gave me an unsafe feeling that mypy and mypy.dmypy code might tangle together.

Note: I don't know how to test these changes (or if they need tests). The self and pytest-fast tests pass but I didn't check others.

The main intent is just to make the code readable.

@studyingegret
Copy link
Author

?? pre-commit wiped my style changes

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2025

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@brianschubert
Copy link
Member

Thanks for your interest in contributing to mypy!

I'm not sure that large cosmetic changes are something the mypy team is interested in. These can be tricky to review, and there are some downsides like damaging the git blame history and the risk of breaking existing behavior (which it look like this change did, going by the test failures). In particular, style changes that aren't compatible with the existing code formatter (black) aren't likely to be accepted, since that creates a future maintanence burden.

If you'd like to contribute (which I very much hope you do!), I'd suggest taking a look at some of the open issues tagged good-first-issue or good-second-issue, which can be great ways to get familiar with the codebase to build up to larger issues. You can also check out CONTRIBUTING.md and the Developer Guides for some general contributing guidelines and tips on where to start.

@brianschubert brianschubert added the pending Issues that may be closed label Aug 9, 2025
@sterliakov
Copy link
Collaborator

Agree with @brianschubert, especially regarding style changes: we employ black specifically to avoid smart and unique code formatting. The goal is quite the opposite, we aim to have a consistent code style everyone is familiar with. Everything else is opinion, and we don't have a common one: I find the original style (one arg per line if all args do not fit on one) much easier to read, because every argument is visually separated from the previous one.

@ilevkivskyi
Copy link
Member

Yeah, sorry, this is not going to happen.

@ilevkivskyi ilevkivskyi closed this Aug 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pending Issues that may be closed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants