Skip to content
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

Proposals: Introduce a formatter #418

Closed
junkmd opened this issue Dec 24, 2022 · 6 comments · Fixed by #427
Closed

Proposals: Introduce a formatter #418

junkmd opened this issue Dec 24, 2022 · 6 comments · Fixed by #427
Labels
drop_py2 dev based on supporting only Python3, see #392
Milestone

Comments

@junkmd
Copy link
Collaborator

junkmd commented Dec 24, 2022

I would like to introduce a formatter for codebases in statically defined modules.

I would like to be able to apply a formatter to PRed codebases by GitHub Actions.
I have wanted to introduce it, but was concerned that it would break the old Python syntax.
But on the drop_py2 plan and its branch, that fear is removed.

Keep in mind, I do not intend to apply this to modules under comtypes.gen that are dynamically generated.
Because it would require third-party libraries for runtime dependencies, which goes against the "pure python" intent of this library.

  • As much as possible, the codegenerator process should be able to generate readable code.
    However, if the code generation process itself becomes more complicated because of this, I do not mind if the generated code is a little difficult to read.
@junkmd junkmd added the drop_py2 dev based on supporting only Python3, see #392 label Dec 24, 2022
@junkmd junkmd added this to the 1.3.0 milestone Dec 24, 2022
@junkmd
Copy link
Collaborator Author

junkmd commented Dec 24, 2022

@cfarrow
@jaraco
@vasily-v-ryabov
And other community participants

I recommend black.

GHA setting will be follows.

# .../.github/workflows/autofmt.yml
on:
  push:

jobs:
    formatter:
        name: formatter
        runs-on: windows-latest
        strategy:
            matrix:
                python-version: [3.7]
        steps:
          - name: Checkout
            uses: actions/checkout@v3
          - name: Set up Python ${{ matrix.python-version }}
            uses: actions/setup-python@v4
            with:
              python-version: ${{ matrix.python-version }}
          - name: Install black
            run: pip install black==22.12.0
          - name: black
            run: python -m black .
          - uses: stefanzweifel/git-auto-commit-action@v4
            with:
              commit_message: Apply Code Formatter Change
              ref: ${{ github.head_ref }}

black has fewer settings than other formatters. Thus, there should be less community exhaustion by having to tussle with setting values.

Almost the only debatable setting value would be line_length.
The default value for black is 88 characters.

I looked at how some well-known projects set theirs up.

Some of them are quite long, like typeshed at 130, some are a little longer, like pipenv at 90, some are unchanged from the default, like django and pandas, and some are 79 to be PEP8-compliant, like attrs.

I am considering not changing the set values from the defaults to avoid controversy.

Any opinion would be appreciated.
If there is no objections, I intend to introduce this by the end of this year.

@junkmd
Copy link
Collaborator Author

junkmd commented Dec 24, 2022

I tried it with my fork.
14ae44d...9baccaa

Since __all__ and __known_symbols__ would be redundant, they would need fmt: off.

@junkmd
Copy link
Collaborator Author

junkmd commented Dec 26, 2022

GHA settings is improved.

on:
  pull_request:
    branches: [drop_py2]  # TODO: add master(main)

jobs:
    formatter:
        name: formatter
        runs-on: windows-latest
        strategy:
            matrix:
                python-version: [3.7]
        steps:
          - name: Checkout
            uses: actions/checkout@v3
          - name: Set up Python ${{ matrix.python-version }}
            uses: actions/setup-python@v4
            with:
              python-version: ${{ matrix.python-version }}
          - name: Install black
            run: pip install black==22.12.0
          - name: format
            run: python -m black .
          - uses: stefanzweifel/git-auto-commit-action@v4
            with:
              commit_message: apply code formatter change
              commit_user_name: github-actions[bot]
              commit_user_email: 41898282+github-actions[bot]@users.noreply.github.com
              commit_author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
              branch: ${{ github.head_ref }}
  • Since it was redundant to trigger on every push, so I made it on every pull_request.
  • I made it explicit that both the committer and the author should be github bot.

I have experimented with this.

1398608...7309055

@jaraco
Copy link
Collaborator

jaraco commented Dec 27, 2022

Please stop pinging me on everything. I specifically turn of email notifications except for mentions so that I can focus on issues that need my specific attention.

@junkmd
Copy link
Collaborator Author

junkmd commented Dec 27, 2022

Sorry. Understood. I will probably ping you to discuss about the packaging(such as #405) and the community operations(such as #396), but otherwise (such as the codebase like this) I would not ping you.

@vasily-v-ryabov
Copy link
Collaborator

Maybe it's nice, but it should be low priority as the project has not so many contributors to have a lot of benefit from it. So it's up to you to implement it or not. Doing such minor things in less number of PRs would be also better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
drop_py2 dev based on supporting only Python3, see #392
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants