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

Add style checker and formatter #3299

Merged
merged 1 commit into from
Mar 15, 2019
Merged

Add style checker and formatter #3299

merged 1 commit into from
Mar 15, 2019

Conversation

ofek
Copy link
Contributor

@ofek ofek commented Mar 14, 2019

What does this PR do?

This adds the ability to easily test for style in our CI and format code locally at will.

Motivation

Consistent code formatting across https://github.com/DataDog/integrations-core and https://github.com/DataDog/integrations-extras.

Implementation

We use a tox plugin that, when dd_check_style is set to true in tox.ini, will dynamically expose 2 new environments:

  1. style - This will check the formatting using a variety of tools. Using the -s/--style flag of ddev test will execute this environment rather than standalone flake8.
  2. format_style - This will format the code for you, often without the need for any additional manual edits. You can run the formatter by using the new -fs/--format-style flag of ddev test.

New checks will have this enabled upon creation.

Style

We now use all of the main style checkers the Python community has coalesced on.

  • black - An opinionated formatter, like JavaScript's prettier and Golang's gofmt.
  • isort - A tool to sort imports lexicographically, by section, and by type. We use the 5 standard sections: __future__, stdlib, third party, first party, and local. datadog_checks is configured as a first party namespace.
  • flake8 - An easy-to-use wrapper around pycodestyle and pyflakes. We select everything it provides and only ignore a few things to give precedence to other tools.
  • bugbear - A flake8 plugin for finding likely bugs and design problems in programs. We use:
    • B001: "Do not use bare except:, it also catches unexpected events like memory errors, interrupts, system exit, and so on. Prefer except Exception:."
    • B003: "Assigning to os.environ doesn't clear the environment. Subprocesses are going to see outdated variables, in disagreement with the current process. Use os.environ.clear() or the env= argument to Popen."
    • B006: "Do not use mutable data structures for argument defaults. All calls reuse one instance of that data structure, persisting changes between them."
    • B007: "Loop control variable not used within the loop body. If this is intended, start the name with an underscore."
    • B301: "Python 3 does not include .iter* methods on dictionaries. The default behavior is to return iterables. Simply remove the iter prefix from the method. For Python 2 compatibility, also prefer the Python 3 equivalent if you expect that the size of the dict to be small and bounded. The performance regression on Python 2 will be negligible and the code is going to be the clearest. Alternatively, use six.iter*."
    • B306: BaseException.message has been deprecated as of Python 2.6 and is removed in Python 3. Use str(e) to access the user-readable message. Use e.args to access arguments passed to the exception."
    • B902: Invalid first argument used for method. Use self for instance methods, and cls for class methods."

Config files

black and isort configuration resides in the now-standardized pyproject.toml at the root. flake8 will follow suit when support lands, but for now resides in its usual .flake8.

Once this is merged we'll start syncing these with https://github.com/DataDog/integrations-extras.

Future

Add the black readme badge for each repo when finished

Also, we may want to eventually:

Notes

There are only 2 things to look out for when formatting:

@codecov
Copy link

codecov bot commented Mar 14, 2019

Codecov Report

Merging #3299 into master will decrease coverage by 7.03%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #3299      +/-   ##
==========================================
- Coverage   84.78%   77.75%   -7.04%     
==========================================
  Files         680       31     -649     
  Lines       35594     1043   -34551     
  Branches     4249       92    -4157     
==========================================
- Hits        30180      811   -29369     
+ Misses       4199      200    -3999     
+ Partials     1215       32    -1183

Copy link
Contributor

@zippolyte zippolyte left a comment

Choose a reason for hiding this comment

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

LGTM

@ofek ofek merged commit 87278a2 into master Mar 15, 2019
@ofek ofek deleted the ofek/style branch March 15, 2019 17:09
@jeffwidman
Copy link
Contributor

jeffwidman commented Jun 20, 2019

❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants