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

AlphabeticalImports is not working as expected. #107

Closed
josecanhelp opened this issue Oct 4, 2019 · 4 comments · Fixed by #113
Closed

AlphabeticalImports is not working as expected. #107

josecanhelp opened this issue Oct 4, 2019 · 4 comments · Fixed by #113
Assignees

Comments

@josecanhelp
Copy link
Contributor

josecanhelp commented Oct 4, 2019

This is how my imports are set up in quicksand right now DeleteOldSoftDeletes.php:

use DateInterval;
use DateTime;
use Exception;
use Illuminate\Config\Repository as Config;
use Illuminate\Console\Command;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;

Tlint should squawk about Log and DB being out of alphabetical order, but running tlint against this file does not catch it.

@loganhenson
Copy link
Contributor

This is working as intended (And should probably be explained along with many others, #106)

It only squaks at the first segment alphabetical ordering, as that is what is required to make the code:
A) Aesthetically pleasing
B) Quick to find something in a top level namespace
C) It was super annoying/pedantic to have to sit and micro-swap stuff like Log and DB (especially when the lint only triggers on a single line, for simplicities sake, and that line is at the top of the imports block)

I'm open to discussion about changing it, but I believe this was the initial justification for the behavior.

@mattstauffer
Copy link
Member

In my opinion it should be full order if possible.

@mattstauffer
Copy link
Member

(My justification for this is: It should be predictable to find anything relative to anything, and full alpha is the most predictable.)

@loganhenson
Copy link
Contributor

ok @mattstauffer (PRs welcome on this Jose, or I will get to it in my list 😄)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants