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

Cumulative lint #24

Merged
merged 22 commits into from
May 1, 2024
Merged

Conversation

Zinkelburger
Copy link
Contributor

@Zinkelburger Zinkelburger commented Apr 26, 2024

Cumulative lint, including changes from my unmerged PRs.

Cherry picked commits from my other lint branch. I believed I have removed most of the conflicting commits.
I'm here Monday/Tuesday, so I'll continue to clean it up .

I still have some suggestions from Thomas to address, but I was able to run this successfully on 227.

The only change I'm really concerned about is making template_args["args"] equal to a string instead of a list. I did test it (and unit test it), but I feel like if this was a breaking change it'd be a headache to discover it.

Signed-off-by: Zinkelburger <[email protected]>
Signed-off-by: Zinkelburger <[email protected]>
- Add tests for evaluator.py, common.py
- Add github workflows to run `mypy`, `black`, `pytest`

Signed-off-by: Zinkelburger <[email protected]>
Signed-off-by: Zinkelburger <[email protected]>
Signed-off-by: Zinkelburger <[email protected]>
Signed-off-by: Zinkelburger <[email protected]>
Signed-off-by: Zinkelburger <[email protected]>
Signed-off-by: Zinkelburger <[email protected]>
- also make self.template_args["args"] a string on line 62 of iperf.py
- I did a unit test to confirm the resulting output is the same, with
  and without a string

Signed-off-by: Zinkelburger <[email protected]>
Signed-off-by: Zinkelburger <[email protected]>
@Zinkelburger Zinkelburger force-pushed the cumulative_lint branch 3 times, most recently from 2dadf21 to bcfb031 Compare April 29, 2024 14:30
@SalDaniele SalDaniele mentioned this pull request Apr 29, 2024
Closed
common.py Show resolved Hide resolved
mypy.ini Show resolved Hide resolved
@SalDaniele
Copy link
Collaborator

LGTM, a couple nits, just ping me when this is ready to merge

@Zinkelburger Zinkelburger force-pushed the cumulative_lint branch 5 times, most recently from 822b67e to 6e831f8 Compare April 29, 2024 20:43
@Zinkelburger Zinkelburger marked this pull request as ready for review April 30, 2024 14:31
@SalDaniele SalDaniele merged commit 3ff5175 into ovn-kubernetes:main May 1, 2024
1 check passed
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