Skip to content

Isort everything#1081

Merged
mssalvatore merged 5 commits into
developfrom
isort-everything
Apr 7, 2021
Merged

Isort everything#1081
mssalvatore merged 5 commits into
developfrom
isort-everything

Conversation

@mssalvatore

@mssalvatore mssalvatore commented Apr 6, 2021

Copy link
Copy Markdown
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

ghost commented Apr 7, 2021

Copy link
Copy Markdown

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
Comment thread 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

@VakarisZ VakarisZ Apr 7, 2021

Copy link
Copy Markdown
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

@shreyamalviya shreyamalviya Apr 7, 2021

Copy link
Copy Markdown
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

@VakarisZ VakarisZ left a comment

Copy link
Copy Markdown
Contributor

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.

shreyamalviya
shreyamalviya reviewed Apr 7, 2021
View reviewed changes
Comment thread monkey/monkey_island/cc/models/test_monkey.py
@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
+1 more reviewer
@shreyamalviya shreyamalviya shreyamalviya approved these changes
Reviewers whose approvals may not affect merge requirements

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