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

Upgrade ni-python-styleguide to version 0.4.1 #253

Merged
merged 10 commits into from
May 3, 2023

Conversation

bkeryan
Copy link
Collaborator

@bkeryan bkeryan commented Apr 29, 2023

What does this Pull Request accomplish?

Update ni-python-styleguide to version 0.4.1.

Fix syntax of noqa comments. To ignore a specific error, you must put a colon after noqa, or else it ignores all errors.

Fix import order in __init__.py.

Remove docstrings from test case functions.

Why should this Pull Request be merged?

Fixes #250

Removes boilerplate docstrings from tests.

What testing has been done?

Ran ni-python-styleguide lint and pytest.

bkeryan added 6 commits April 28, 2023 19:54
Signed-off-by: Brad Keryan <[email protected]>
Signed-off-by: Brad Keryan <[email protected]>
Signed-off-by: Brad Keryan <[email protected]>
Signed-off-by: Brad Keryan <[email protected]>
Signed-off-by: Brad Keryan <[email protected]>
@bkeryan bkeryan changed the title Upgrade ni-python-styleguide Upgrade ni-python-styleguide to version 0.4.1 Apr 29, 2023
@bkeryan
Copy link
Collaborator Author

bkeryan commented Apr 29, 2023

@mshafer-NI I tried to fix the noqa comments by removing them and then running acknowledge-existing-violations, but this didn't work out: ni/python-styleguide#125

@mshafer-NI
Copy link
Collaborator

mshafer-NI commented May 1, 2023

@mshafer-NI I tried to fix the noqa comments by removing them and then running acknowledge-existing-violations, but this didn't work out: ni/python-styleguide#125

@bkeryan
Please try again using 0.4.1

@bkeryan
Copy link
Collaborator Author

bkeryan commented May 1, 2023

@mshafer-NI I tried to fix the noqa comments by removing them and then running acknowledge-existing-violations, but this didn't work out: ni/python-styleguide#125

@bkeryan Please try again using 0.4.1

I upgraded to 0.4.1 before I tried to do this.

I didn't know about fix --aggressive. I'll try that.

@bkeryan
Copy link
Collaborator Author

bkeryan commented May 3, 2023

I didn't know about fix --aggressive. I'll try that.

ni/python-styleguide#129

@bkeryan
Copy link
Collaborator Author

bkeryan commented May 3, 2023

checks-nimg was failing for two reasons:

  • I forgot to upgrade ni-python-styleguide in the generator's pyproject.toml.
  • It wasn't following the standard test directory structure (tests/acceptance/test_.py, tests/unit/tests_.py, etc.) so the exclusion didn't work.

- Copy `test_assets_directory` fixture from nidaqmx-python.
- Replace `tmpdir` fixture with `tmp_path_factory`, which uses pathlib

Signed-off-by: Brad Keryan <[email protected]>
@bkeryan bkeryan requested a review from dixonjoel May 3, 2023 21:30
@bkeryan bkeryan merged commit 606b227 into main May 3, 2023
@bkeryan bkeryan deleted the users/bkeryan/upgrade-styleguide branch May 3, 2023 21:42
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.

noqa comments are overly lenient
3 participants