-
-
Notifications
You must be signed in to change notification settings - Fork 587
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
Ensure that split_on_trailing_comma works with as
imports
#2340
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2340 +/- ##
=======================================
Coverage 99.12% 99.12%
=======================================
Files 40 40
Lines 3096 3096
Branches 787 787
=======================================
Hits 3069 3069
Misses 15 15
Partials 12 12 |
test_input = "from lib import (a as b,)" | ||
expected_output = """from lib import a as b | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's expected for multiple as
imports an a trailing comma, e.g. from lib import (a as b, c as d,)
?
from lib import a as b
from lib import c as d
or
from lib import (
a as b,
c as d,
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current code will do:
from lib import a as b
from lib import c as d
@DanielNoord this Django integration test still failimg, did you have time to check if it is related or not to these black changes? |
Like I told you in the message: by changing the black style django is no longer conformant. The tests will fail until we release and they reformat their repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DanielNoord ok, then we can temporarily disable it and put a comment to re-enable when they upgrade to 6.0.0+
@staticdev There we go. I double checked and the offending line is:
I think this is fine to merge now :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DanielNoord LGTM, thanks.
Closes #2338
Took the tests as recommended in #2339
I don't think we should fix the tests by disabling the new profile flag, that would mean that our tests work but anybody using the profile would still get broken imports.
Instead, let's try to fix
config.split_on_trailing_comma
itself. I believe this does this.