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

why this style? #104

Closed
karolpawlowski opened this issue Oct 17, 2019 · 10 comments
Closed

why this style? #104

karolpawlowski opened this issue Oct 17, 2019 · 10 comments

Comments

@karolpawlowski
Copy link

Consider the following example which causes a merge conflict:

# developer 1
-from typing import Dict, List
+from typing import Any, Dict, List
# developer 2
-from typing import Dict, List
+from typing import Dict, List, Tuple

no conflict with the style enforced by reorder-python-imports:

+from typing import Any
 from typing import Dict
 from typing import List
+from typing import Tuple

Why not

from typing import (
+    Any,
     Dict,
     List,
+    Tuple,
)

?

It also doesn't conflicts and you have shorter, maybe even more readable lines.

@asottile
Copy link
Owner

you can't # noqa: F401 individual imports

@asottile
Copy link
Owner

also the bikeshed is already painted :)

@asottile
Copy link
Owner

other [-]s from the proposal:

  • it's more lines
  • going from 1 import to 2 is a quite non-minimal diff (-1,+4) instead of (+1)

@con-f-use
Copy link

con-f-use commented Oct 1, 2022

However, a flag to disable the conversion from from typing import Dict, List, Tuple into imports on individual lines, and a similar one to disable import os, sys, re conversion would be appreciated. What is your stance on that, Anthony? I assume you thought about it and opted to not include such flags. I'd love to hear why.

Personally, I'd like to have the last one especially. In my team, people know the standard library modules and do not want them wordily listed out, wasting vertical space at the start of every file.

@asottile
Copy link
Owner

asottile commented Oct 1, 2022

I don't care about your personal style and neither does the tool, if you don't like what it does use something else

you're also missing the point

@con-f-use
Copy link

Rude.

@asottile
Copy link
Owner

asottile commented Oct 1, 2022

no you, you're commenting on a 3 year old closed issue asking to add a bunch of flags to go against the core design principles of the tool -- ones which prevent you from having merge conflicts.

@asottile
Copy link
Owner

asottile commented Oct 1, 2022

in case you missed the readme which uses almost exactly your example as rationale -- https://github.com/asottile/reorder_python_imports#why-this-style or PEP8 which discourages your second proposal https://peps.python.org/pep-0008/#imports

@con-f-use
Copy link

con-f-use commented Oct 1, 2022

Fair enough, thanks for the answer. I do appreciate the tools existence and your time, wish that would have been your first reply.
I didn't open an additional issue, because this one was close enough and "why this style?" seemed sufficiently open a title inviting discussion in lieu of an official designated area.
I just thought, not everyone needs to avoid merge conflicts that consequently and being open for a bit of personal style might be possible. You do have very specific and very well-founded ideas on how an issue tracker is supposed to be used and what exactly your tool want to achieve, based on a long and distinguished career. Just please remember that is not the case for everyone and not everyone shares the same ideas or has that much specific experience. In no case is that a reason to tear someone a new one. Correct me if I'm wrong, but I don't think I was particularly disrespectful or committed some sort of universally accepted faux pas.

@asottile
Copy link
Owner

asottile commented Oct 1, 2022

if you're interpreting "no" as "tearing you a new one" you need to seriously reconsider your sensitivity

Repository owner locked as off-topic and limited conversation to collaborators Oct 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants