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

fix a typo caught by lintian #75

Merged
merged 1 commit into from Jul 21, 2022

Conversation

emollier
Copy link
Contributor

While pulling the last release of PDF-Table into Debian, the linter
"lintian" caught a typo. I thought you might be interested in the
resulting patch.

Signed-off-by: Étienne Mollier emollier@debian.org

While pulling the last release of PDF-Table into Debian, the linter
"lintian" caught a typo.

Signed-off-by: Étienne Mollier <emollier@debian.org>
Copy link
Owner

@PhilterPaper PhilterPaper left a comment

Choose a reason for hiding this comment

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

Yes indeed, you caught a typo from one of the earlier maintainers! I can't believe that I missed it, looking over the file so many times. Thanks!

@PhilterPaper PhilterPaper merged commit e4ac414 into PhilterPaper:master Jul 21, 2022
@mdeweerd
Copy link

Codespell finds a few more:

$ codespell .
./Changes:57: noticable ==> noticeable
./examples/border_rules.pl:342: optio ==> option
./examples/chess.pl:20: catpure ==> capture
./INFO/Changes_2019:24: commmands ==> commands
./INFO/Changes_2019:31: warnind ==> warning
./INFO/Changes_2019:40: accomodate ==> accommodate
./INFO/Changes_2019:132: Accidently ==> Accidentally
./INFO/Changes_2019:174: wich ==> which
./INFO/Changes_2019:177: ot ==> to, of, or
./INFO/Changes_2019:180: immidiately ==> immediately
./INFO/Changes_2019:191: overlaping ==> overlapping
./INFO/Changes_2019:197: overiding ==> overriding
./INFO/Changes_2019:234: spliting ==> splitting
./INFO/Changes_2019:248: beeing ==> being, been
./INFO/Changes_2019:260: incorect ==> incorrect
./INFO/Changes_2019:262: remeber ==> remember
./INFO/Table.html:817: asumes ==> assumes
./lib/PDF/Table.pm:1273: contination ==> continuation
./lib/PDF/Table/Table.pod:887: asumes ==> assumes

I recommend pre-commit to help avoid some of this. Here is one I amended from my projects. It includes code formatting as well.

precommit-conf.zip

To try it, install pre-commit ( pip3 install pre-commit ), add the configuration file to the root of the project, and run pre-commit run -a .
To have the tool run on commit, do pre-commit install.

The configuration file I share will make several changes, some of which you may not like, so make sure you try it in a copy that you can revert or discard.

@PhilterPaper
Copy link
Owner

Thanks for the list. Some of them were mine. The "optio" is from Ipsum Lorem text, so I left it alone.

I'll have to look at the lintian and codespell tools, as well as precommit. It looks like it tried to clean up the indentation in Changes, which I didn't want.

@emollier
Copy link
Contributor Author

emollier commented Jul 22, 2022 via email

@mdeweerd
Copy link

I can update the configuration file so that it excludes the changelog as wel as specific words.

@mdeweerd
Copy link

precommit-conf.zip

I think that the changes to "Changes" only concern line endings and possibly also tabs. Tabs being dependant on the editor settings, I prefer using spaces where possible.

I updated the pre-commit configuration to exclude "optio" from the codespell report, however I did not exclude noticable because that seems to be an incorrect spelling (https://www.askdifference.com/noticeable-vs-noticable/ ).

@PhilterPaper
Copy link
Owner

OK, I'll ignore lintian since I'm not working with Debian.

If I end up deciding to use precommit-conf, where does it go? Do I just put it in .github/workflows, alongside test.yml, or does it go somewhere else, or is it to be merged into test.yml?

@mdeweerd
Copy link

mdeweerd commented Jul 22, 2022

The pre-commit configuration goes at the root of the git repository using the file name ".pre-commit-configuration.yaml".

You enable the pre-commit hook by executing 'pre-commit install' in a clone of the repository. Then it is executed on commit.

You can disable the "automatic run" of some pre-commit hooks by adding "stages: manual":

  - repo: https://github.com/codespell-project/codespell
    rev: v2.1.0
    hooks:
      - id: codespell
        stages: [manual]  # Require to run this manually
        args:
          - --ignore-words-list=optio

Suppose that you add that to codespell, then you can run codespell using pre-commit run --hook-stage=manual -a codespell but it would not run on commit.

It's possible to add pre-commit to the continuous integration, and even to add test exécution to pre-commit.

This is a sample job in tests.yml to execute pre-commit which could surely be simplified because this is not a python project:

jobs:
  pre-commit:
    runs-on: "ubuntu-latest"
    name: Pre-commit
    steps:
      - name: Check out the repository
        uses: actions/checkout@v2.3.4

      - name: Set up Python ${{ env.DEFAULT_PYTHON }}
        uses: actions/setup-python@v2.2.1
        with:
          python-version: ${{ env.DEFAULT_PYTHON }}

      - name: Upgrade pip
        run: |
          pip install --constraint=.github/workflows/constraints.txt pip
          pip --version

      - name: Install Python modules
        run: |
          pip install --constraint=.github/workflows/constraints.txt pre-commit

      - name: Run pre-commit on all files
        run: |
          pre-commit run --all-files --show-diff-on-failure --color=always

I managed to run the tests by adding this to the pre-commit configuration:

    hooks:
      - id: prove
        name: prove
        entry: bash -c "prove -l -Q t/*.t"
        language: perl
        always_run: true
        files: ^$

@emollier emollier deleted the lintian-typo branch December 18, 2022 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants