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 flake8 reported issues, run tests #36

Closed
wants to merge 1 commit into from
Closed

fix flake8 reported issues, run tests #36

wants to merge 1 commit into from

Conversation

sandrobonazzola
Copy link
Contributor

@sandrobonazzola sandrobonazzola commented Oct 24, 2022

Cleaned up the code, added automated tests.

Not adding all the unit tests at once as they are failing on my laptop. I'll enable all tests in a different PR once this is in.

Coverage report: 25%

coverage.py v6.5.0, created at 2022-10-24 13:57 +0000

Module statements missing excluded coverage
drop_buffer_cache.py 33 15 0 55%
fallocate.py 35 17 0 51%
parser_data_types.py 49 27 0 45%
smallfile.py 1450 1181 0 19%
smf_test_params.py 98 69 0 30%
sync_files.py 47 36 0 23%
yaml_parser.py 146 57 0 61%
Total 1858 1402 0 25%

coverage.py v6.5.0, created at 2022-10-24 13:57 +0000

Signed-off-by: Sandro Bonazzola [email protected]

Copy link
Member

@portante portante left a comment

Choose a reason for hiding this comment

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

Was black run on this as well? If so, it might be best to have one commit to see what black did, one for what isort did, and then finally the by-hand changes.

That way, I can review the by-hand changes that were made separate from the automated changes.

@sandrobonazzola
Copy link
Contributor Author

No, didn't run black. Run flake8 and isort and manually fixed stuff reported there.

@portante
Copy link
Member

No, didn't run black. Run flake8 and isort and manually fixed stuff reported there.

Have you considered using black? It'd make these changes much easier to consume in review.

@sandrobonazzola
Copy link
Contributor Author

pushed an additional commit with black run.

@sandrobonazzola sandrobonazzola changed the title fix flake8 and isort reported issues, run tests fix black reported issues, run tests Oct 24, 2022
@sandrobonazzola
Copy link
Contributor Author

Had to drop both isort and flake8 as they don't agree with black.

@portante
Copy link
Member

Here is a PR which just applies black and isort without any other changes, #37.

@portante
Copy link
Member

If we take that PR in first, and then rebase this PR on that, we'll have just the fixes you have applied which will be a bit easier to review.

@sandrobonazzola
Copy link
Contributor Author

ok, go ahead merging #37 and I'll rebase.

@sandrobonazzola sandrobonazzola changed the title fix black reported issues, run tests fix pyflakes reported issues, run tests Oct 25, 2022
@sandrobonazzola sandrobonazzola changed the title fix pyflakes reported issues, run tests fix flake8 reported issues, run tests Oct 25, 2022
@sandrobonazzola sandrobonazzola deleted the master branch October 25, 2022 10:12
@sandrobonazzola
Copy link
Contributor Author

moved to #38 due to master/main rename

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