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

NH-11537 NH-11539 add tox linting and formatting #80

Merged
merged 18 commits into from
Nov 22, 2022

Conversation

tammy-baylis-swi
Copy link
Contributor

This adds NH Python code linting and formatting ways similar to OTel Python's as far as I understand right now:

  1. linting and formatting with tox (see CONTRIBUTING diff in this PR for how to run)
  2. GH workflow check with CodeQL

I've ported over a lot of the OTel Python settings for what to specifically exclude, how to format the results, which rules to disable, etc. I've not really included anything from AO Python.

black and isort can perform some autofixing. There is no autofixing available with flake8 and there probably never will be. I think it's the same for pylint but I can't find it in its own official docs.

This doesn't enforce formatting with any hooks nor upon-PR action triggers yet. I think I want to do this:

  1. Merge this PR if it looks good for a first iteration
  2. In a 2nd PR, make actual fixes to the code formatting! It'll be the autofix results plus some manual fixes identified so far and there's quite a few.
  3. In a 3rd PR, when all linting/formatting passes, add a new GH workflow to run linting/formatting like Otel Python does with Core Repo tests. It will run at one or more of push and PR, and will error if even a single lint/format issue identified.

I also think I'll be able to test the CodeQL GH workflow once this has been merged to main.

Please let me know what you think! 😺

@tammy-baylis-swi tammy-baylis-swi force-pushed the NH-11537-add-linting-formatting branch from 6ac304b to 0374bea Compare November 22, 2022 00:40
Copy link
Contributor

@cheempz cheempz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @tammy-baylis-swi for the analysis and writeup as always. Note that CodeQL may not be available to us yet per separate Jira/email thread that I just cc'd you in.

@tammy-baylis-swi tammy-baylis-swi merged commit 32c18e2 into main Nov 22, 2022
@tammy-baylis-swi tammy-baylis-swi deleted the NH-11537-add-linting-formatting branch November 22, 2022 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants