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

Configured isort to use its "django" profile. #14594

Closed
wants to merge 1 commit into from

Conversation

adamchainz
Copy link
Sponsor Member

isort 5 added profiles to represent common configurations, including one copying Django’s config ( https://pycqa.github.io/isort/docs/configuration/profiles.html ), so we no longer need to specify the options directly.

isort 5 added profiles to represent common configurations, including one copying Django’s config ( https://pycqa.github.io/isort/docs/configuration/profiles.html ), so we no longer need to specify the options directly.
@adamchainz adamchainz requested a review from apollo13 July 5, 2021 10:07
@felixxm
Copy link
Member

felixxm commented Jul 5, 2021

@adamchainz Thanks for this patch 👍 However I don't think that we should "import" our configuration from the 3rd-party package. It would create a circular logic dependency and it's confusing. isort's "django" profile is rather for others not for Django itself 🤷 . Let's see what other think.

@ngnpope
Copy link
Member

ngnpope commented Jul 5, 2021

Also, when we adopt black won't we want to use isort's black profile over its django profile? That'll be amusing… 😄

@apollo13
Copy link
Member

apollo13 commented Jul 5, 2021

I have no strong feelings, the "django" profile is probably only relevant till we can drop in "black" (sooooon ;)). I somewhat agree with Mariusz that it at least feels weird to pull our configuration from someone else (but all in all no strong feelings). What if we were to change the config in the future? Then we'd have to ask isort to release a new release for us :D

@adamchainz
Copy link
Sponsor Member Author

Also, when we adopt black won't we want to use isort's black profile over its django profile? That'll be amusing… 😄

Yes, absolutely.

What if we were to change the config in the future? Then we'd have to ask isort to release a new release for us :D

We can override it on a per-rule basis. But with so many years without change, plus changing to the Black profile on the horizon, it seems very unlikely to change.

@felixxm
Copy link
Member

felixxm commented Jul 5, 2021

But with so many years without change ...

I still don't see any benefits, grumbling... 👴 😉 Sure we can do this, but why? just because we can 🤔 that's not a rationale (my 2️⃣ ¢.)

@apollo13
Copy link
Member

apollo13 commented Jul 5, 2021 via email

@adamchainz
Copy link
Sponsor Member Author

I'm happy whether you close or merge. One small improvement is that the shorter config makes it easier for others looking to make packages to adopt the same style. Also, moving the config to pyproject.toml is good long term as the ecosystem moves to configuring all tools through that file.

@felixxm felixxm removed the request for review from apollo13 July 6, 2021 08:31
@felixxm
Copy link
Member

felixxm commented Jul 6, 2021

@adamchainz Thanks again 🤗

Closing due to lack of consensus.

@felixxm felixxm closed this Jul 6, 2021
@adamchainz adamchainz deleted the isort_format branch April 22, 2024 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants