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.

Sign up for GitHub

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

Jump to bottom

Isort everything #1081

Merged
merged 5 commits into from
Apr 7, 2021
Merged

Isort everything #1081

merged 5 commits into from
Apr 7, 2021

Conversation

mssalvatore
Copy link
Collaborator

What does this PR do?

Resolves #990

Adds an isort configuration, isort check to travis, and runs isort v5.8.0 on all python code.

PR Checklist

  • Have you added an explanation of what your changes do and why you'd like to include them?
  • Is the TravisCI build passing?
  • Was the CHANGELOG.md updated to reflect the changes?
  • Was the documentation framework updated to reflect the changes?

Testing Checklist

  • Added relevant unit tests?
  • Have you successfully tested your changes locally? Elaborate:

    Tested by running unit tests and blackbox tests

  • If applicable, add screenshots or log transcripts of the feature working

@mssalvatore mssalvatore requested review from VakarisZ and shreyamalviya and removed request for VakarisZ April 6, 2021 23:22
@ghost
Copy link

ghost commented Apr 7, 2021

DeepCode failed to analyze this pull request

Something went wrong despite trying multiple times, sorry about that.
Please comment this pull request with "Retry DeepCode" to manually retry, or contact us so that a human can look into the issue.

VakarisZ
VakarisZ reviewed Apr 7, 2021
View reviewed changes
pyproject.toml
Comment on lines +10 to +14
multi_line_output = 3
include_trailing_comma = true
force_grid_wrap = 0
use_parentheses = true
ensure_newline_before_comments = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on this, wouldn't this be enough?

[tool.isort]
profile = "black"
multi_line_output = 3

Copy link
Contributor

Choose a reason for hiding this comment

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

It won't pick up our black config. It'll use a predefined one (which doesn't match ours). See this.

VakarisZ
VakarisZ approved these changes Apr 7, 2021
View reviewed changes
Copy link
Contributor

@VakarisZ VakarisZ left a comment

Choose a reason for hiding this comment

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

Approved, but take a look at my comment.

@mssalvatore mssalvatore merged commit 1d7211e into develop Apr 7, 2021
@mssalvatore mssalvatore deleted the isort-everything branch April 7, 2021 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VakarisZ VakarisZ VakarisZ approved these changes

@shreyamalviya shreyamalviya shreyamalviya approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add isort to CI
3 participants
@mssalvatore @VakarisZ @shreyamalviya