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

Lint fixes and enhancements #779

Merged
merged 4 commits into from
Mar 27, 2024

Conversation

jbaptperez
Copy link
Contributor

@jbaptperez jbaptperez commented Sep 21, 2023

A couple of modifications:

  1. Fixes lint issues:
    1. flake8,
    2. pylint (factorizes duplicated code into a pytests/common.py file),
  2. Fixes CONTRIBUTING.md:
    1. Typos,
    2. Updates URL of signing request documentation (--signoff instead of --gpg-sign),
    3. Formats Markdown.

Lint fixing commits are independents.

As I added the pytests/common.py source file to factorise some duplicated code, the pre-commit run --all-files command starts require an initial call to export PYTHONPATH=$PWD where $PWD is the root directory of the repository.

Is really # pylint: disable=import-error a correct way to handle this kind of issue?
I did not find a proper and portable way to solve this problem directly in .pylintrc, but I'm open to suggestions.

@jbaptperez jbaptperez force-pushed the lint-fixes-and-enhancements branch 2 times, most recently from 8e6bc3e to ec4b9ea Compare September 23, 2023 10:34
Copy link
Collaborator

@p12tic p12tic left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

In the documentation, please keep line length to around 100 chars or less.

The unit tests have migrated from pylint to unittest, so these changes are probably no longer relevant.

I understand a lot of time have passed, so you probably won't have time to go back to the PR. Maybe someone else can use the review to help extract useful stuff from the PR.

The initial link was related to GPG signature in commits (--gpg-sign or
-S git-commit option) as the required signature is actually a
"Signed-off-by" trailer at the end of the commit message (--signoff or
-s git-commit option).

Signed-off-by: Jean-Baptiste Perez <[email protected]>
Signed-off-by: Jean-Baptiste Perez <[email protected]>
Signed-off-by: Jean-Baptiste Perez <[email protected]>
@jbaptperez jbaptperez force-pushed the lint-fixes-and-enhancements branch from ec4b9ea to 81d81fb Compare March 17, 2024 18:32
@jbaptperez
Copy link
Contributor Author

jbaptperez commented Mar 17, 2024

@p12tic

Thanks for the PR.

In the documentation, please keep line length to around 100 chars or less.

Done.

The unit tests have migrated from pylint to unittest, so these changes are probably no longer relevant.

I understand a lot of time have passed, so you probably won't have time to go back to the PR. Maybe someone else can use the review to help extract useful stuff from the PR.

I removed outdated fixes, rebased my branch onto main and updated the PR.

However, the pytest to unitest and ruff migrations look not to be completed and I think README.md and CONTRIBUTING.md (or even setup.py) have to be updated accordingly.

@p12tic
Copy link
Collaborator

p12tic commented Mar 27, 2024

However, the pytest to unitest and ruff migrations look not to be completed and I think README.md and CONTRIBUTING.md (or even setup.py) have to be updated accordingly.

Thanks, indeed..

@p12tic p12tic merged commit 07128f6 into containers:main Mar 27, 2024
8 checks passed
@jbaptperez jbaptperez deleted the lint-fixes-and-enhancements branch March 28, 2024 20:56
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.

2 participants