Skip to content

Conversation

@jessp01
Copy link
Contributor

@jessp01 jessp01 commented Sep 11, 2025

_typos.toml is used to discard false positives in email headers:

error: `Forr` should be `For`
--> tests/data/spec_no_trailing_newline/patch0.patch:2:26 |
2 | From: =?UTF-8?q?Nikola=20Forr=C3=B3?= <[email protected]>
|                          ^^^^

On the subject of pre-commit hooks, might I suggest removing https://github.com/pre-commit/mirrors-prettier?
pre-commit/mirrors-prettier was actually archived back in Apr 11, 2024.
Further, on my Rocky8 machine, with both Python 3.9 and 3.12, it kept on failing to retrieve the nodejs tar it claimed to need:

$ git commit -a
[INFO] Installing environment for https://github.com/pre-commit/mirrors-prettier.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('/usr/bin/python3.12', '-mnodeenv', '--prebuilt', '--clean-src', '/home/jesse/.cache/pre-commit/repo1a7gxoya/node_env-default')
return code: 1
stdout: (none)
stderr:
     * Install prebuilt node (24.8.0) .Incomplete read while reading from https://nodejs.org/download/release/v24.8.0/node-v24.8.0-linux-x64.tar.gz - IncompleteRead(54483458 bytes read, 4401371 more expected)
    Incomplete read while reading from https://nodejs.org/download/release/v24.8.0/node-v24.8.0-linux-x64.tar.gz - IncompleteRead(54417858 bytes read, 4466971 more expected)
    Incomplete read while reading from https://nodejs.org/download/release/v24.8.0/node-v24.8.0-linux-x64.tar.gz - IncompleteRead(56590530 bytes read, 2294299 more expected)
    
    Traceback (most recent call last):
      File "<frozen runpy>", line 198, in _run_module_as_main
      File "<frozen runpy>", line 88, in _run_code
      File "/home/jesse/.local/lib/python3.12/site-packages/nodeenv.py", line 1548, in <module>
        main()
      File "/home/jesse/.local/lib/python3.12/site-packages/nodeenv.py", line 1130, in main
        create_environment(env_dir, args)
      File "/home/jesse/.local/lib/python3.12/site-packages/nodeenv.py", line 1006, in create_environment
        install_node(env_dir, src_dir, args)
      File "/home/jesse/.local/lib/python3.12/site-packages/nodeenv.py", line 763, in install_node
        install_node_wrapped(env_dir, src_dir, args)
      File "/home/jesse/.local/lib/python3.12/site-packages/nodeenv.py", line 786, in install_node_wrapped
        download_node_src(node_url, src_dir, args)
      File "/home/jesse/.local/lib/python3.12/site-packages/nodeenv.py", line 618, in download_node_src
        dl_contents = _download_node_file(node_url)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/home/jesse/.local/lib/python3.12/site-packages/nodeenv.py", line 610, in _download_node_file
        raise e
      File "/home/jesse/.local/lib/python3.12/site-packages/nodeenv.py", line 602, in _download_node_file
        return io.BytesIO(urlopen(node_url).read())
                          ^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib64/python3.12/http/client.py", line 495, in read
        s = self._safe_read(self.length)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib64/python3.12/http/client.py", line 644, in _safe_read
        raise IncompleteRead(data, amt-len(data))
    http.client.IncompleteRead: IncompleteRead(56590530 bytes read, 2294299 more expected)

On my Debian GNU/Linux 13 (trixie) machine:

$ python --version
Python 3.13.5
$ git commit -a
[INFO] Installing environment for https://github.com/pre-commit/mirrors-prettier.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('/usr/bin/python3', '-mnodeenv', '--prebuilt', '--clean-src', '/home/jesse/.cache/pre-commit/repojzxdfadf/node_env-default')
return code: 1
stdout: (none)
stderr:
    /usr/lib/python3/dist-packages/nodeenv.py:759: SyntaxWarning: invalid escape sequence '\d'
      VERSION_RE = re.compile('\d+\.\d+\.\d+')
    /usr/lib/python3/dist-packages/nodeenv.py:960: SyntaxWarning: invalid escape sequence '\.'
      local re="[a-zA-Z0-9\.\-]+@[0-9]+\.[0-9]+\.[0-9]+([\+\-][a-zA-Z0-9\.\-]+)*"
    Traceback (most recent call last):
      File "<frozen runpy>", line 198, in _run_module_as_main
      File "<frozen runpy>", line 88, in _run_code
      File "/usr/lib/python3/dist-packages/nodeenv.py", line 24, in <module>
        import pipes
    ModuleNotFoundError: No module named 'pipes'

Considering that this code base is basically pure Python and we use:

  - repo: https://github.com/psf/black
    rev: 25.1.0
    hooks:
      - id: black

I'm not sure what value pre-commit/mirrors-prettier actually adds but if there's something it checks the other hooks do not address, I'd recommend finding a maintained alternative.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes

Hello @jessp01, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances code quality by introducing an automated typo-checking mechanism. It integrates the typos tool as a pre-commit hook, ensuring that common spelling errors are caught early in the development cycle. A custom configuration is included to prevent typos from flagging legitimate patterns found in email headers as errors.

Highlights

  • New typos pre-commit hook: Integrates the typos spell checker into the pre-commit workflow to automatically identify and fix typos before commits.
  • typos configuration for email headers: A _typos.toml configuration file is added to instruct typos to ignore specific patterns in UTF-8 encoded email headers, preventing false positives.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a typos pre-commit hook, which is a valuable addition for maintaining code quality. However, I've identified two critical configuration issues that will prevent the hook from functioning correctly. The specified version for the typos hook does not exist, and the configuration file _typos.toml uses an invalid structure. My review provides specific suggestions to resolve these problems.

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/fdb6b9e7d652470ba5375196ec5b147b

✔️ pre-commit SUCCESS in 2m 04s
✔️ specfile-tests-rpm-deps SUCCESS in 1m 47s
✔️ specfile-tests-pip-deps SUCCESS in 1m 42s

@jessp01
Copy link
Contributor Author

jessp01 commented Sep 11, 2025

Code Review

This pull request introduces a typos pre-commit hook, which is a valuable addition for maintaining code quality. However, I've identified two critical configuration issues that will prevent the hook from functioning correctly. The specified version for the typos hook does not exist My review provides specific suggestions to resolve these problems.

This is false (see https://github.com/packit/specfile/pull/490/files/4c5bea2c65ce9a7eddcf64344322db829c88661b#r2341657423).
Apparently, this Gemini automation is unable to edit its past comments (rather ridiculous, IMHO but nonetheless, here we are...).

@nforro
Copy link
Member

nforro commented Sep 16, 2025

On the subject of pre-commit hooks, might I suggest removing https://github.com/pre-commit/mirrors-prettier?

Considering that this code base is basically pure Python and we use:

  - repo: https://github.com/psf/black
    rev: 25.1.0
    hooks:
      - id: black

prettier and black have different use-cases, black is not a replacement for prettier. We don't need black, we have actually replaced it with ruff a while back.

Copy link
Member

@nforro nforro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@nforro nforro force-pushed the add-typos-pre-commit-check branch from e61a595 to bfa06da Compare September 16, 2025 07:07
@nforro nforro moved this from new to in-review in Packit Kanban Board Sep 16, 2025
@nforro nforro added the mergeit Merge via Zuul label Sep 16, 2025
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/9c6bdc6dcb8e4a9ba4320589aba355ef

✔️ pre-commit SUCCESS in 2m 11s
✔️ specfile-tests-rpm-deps SUCCESS in 1m 58s
✔️ specfile-tests-pip-deps SUCCESS in 1m 54s

@jessp01
Copy link
Contributor Author

jessp01 commented Oct 6, 2025

Hi @nforro ,

I noticed this still wasn't merged, although you approved it and added the mergeit label (3 weeks ago). Is something missing?

Cheers,

@nforro
Copy link
Member

nforro commented Oct 7, 2025

regate

@nforro
Copy link
Member

nforro commented Oct 7, 2025

Sorry, it seems Zuul had some issues and didn't pick it up. Let me rebase and try again.

_typos.toml is used to discard false positives in email headers:

error: `Forr` should be `For`
--> tests/data/spec_no_trailing_newline/patch0.patch:2:26
|
2 | From: =?UTF-8?q?Nikola=20Forr=C3=B3?= <[email protected]>
|                          ^^^^
error: `SOURCEN` should be `SOURCE`
    ╭▸ specfile/spec_parser.py:241:46
    │
241 │             # source references: %SOURCEN, %{SOURCEN}, %{S:N}
    ╰╴                                             ━━━━━━━
@nforro nforro force-pushed the add-typos-pre-commit-check branch from bfa06da to 02f922e Compare October 7, 2025 08:51
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/bc617d8e774349b28bd2a2fe7f0cdeb6

✔️ pre-commit SUCCESS in 2m 05s
✔️ specfile-tests-rpm-deps SUCCESS in 1m 53s
✔️ specfile-tests-pip-deps SUCCESS in 1m 47s

@nforro
Copy link
Member

nforro commented Oct 7, 2025

regate

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).
https://softwarefactory-project.io/zuul/t/packit-service/buildset/f8f0e64a0f38424e8aac8324d10c3971

✔️ pre-commit SUCCESS in 2m 06s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 61e261c into packit:main Oct 7, 2025
46 of 49 checks passed
@github-project-automation github-project-automation bot moved this from in-review to done in Packit Kanban Board Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mergeit Merge via Zuul

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants