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

Added ruleset for alphabetizing import statements #33

Merged
merged 1 commit into from Mar 11, 2016
Merged

Conversation

Haud
Copy link
Contributor

@Haud Haud commented Feb 22, 2016

Proposal; rule that I used to abide by in C#. I think it has value here in organizing imports, as I don't think we have ever had any rulesets for imports in Objective-C. Thoughts? Alternatives? This seems pretty simple.

@hkellaway
Copy link

i'd perhaps put a qualifier i.e. for lack of better ordering, make import statements alphabetical

@Haud
Copy link
Contributor Author

Haud commented Feb 22, 2016

I could, but that feels like an unnecessary qualifier to me -- almost as if we're giving up. If there are other, better ordering options, we should propose them and implement them. I think this is a fine standard, but I'm open to other suggestions based on feedback if people feel strongly that there are better, more concise ways to order imports.

@prolificphotis
Copy link

Are there tools to do this automatically? I wouldn't put it in unless there are.

Photis Patriotis
VP of Engineering
Prolific Interactive
BK // 45 Main Street, Suite 1006 | Brooklyn, NY 11201
SF // 995 Market Street, Suite 1401 | San Francisco, CA 94103
m: 215.480.9759 | prolificinteractive.com | @weareprolific

On Feb 22, 2016, at 7:18 AM, Harlan Kellaway notifications@github.com wrote:

i'd perhaps put a qualifier i.e. for lack of better ordering, make import statements alphabetical


Reply to this email directly or view it on GitHub.

@Haud
Copy link
Contributor Author

Haud commented Feb 22, 2016

@prolificphotis as of my knowledge, no. Even so, I still think it should be part of the style guide and then enforced by any third-party tools. It's similar to our bracket syntax rule, etc.

@liberatevillage
Copy link

👍

@prolificphotis @Haud AppCode has a feature that does this for you I believe

@ThibaultKlein
Copy link
Collaborator

I was used to organize based on the length of the import and build a half pyramid:

import UIKit
import RealmSwift
import Foundation
import AFNetworking
import ReactiveCocoa

@Haud
Copy link
Contributor Author

Haud commented Feb 22, 2016

@ThibaultKlein lol that's kinda cool, but I think a bit more nonsensical :)

@Haud
Copy link
Contributor Author

Haud commented Feb 24, 2016

Since we're on the discussion of automating rules like this, I've created a fork of Swiftlint and added a rule for this: https://github.com/prolificinteractive/SwiftLint/pull/1

If we were to merge this, we can have this custom rule added to our swiftlint process as well.

@ThibaultKlein
Copy link
Collaborator

conflicts, but 👍

@ppatriotis
Copy link

Please link to the swiftlint mod too

@Haud
Copy link
Contributor Author

Haud commented Feb 24, 2016

@prolificphotis what do you mean? I included the PR for the lint addition

@ppatriotis
Copy link

@Haud I just mean I think we should link to it for now. in the styleguide

@Haud
Copy link
Contributor Author

Haud commented Feb 24, 2016

@prolificphotis I think that it would make more sense to do that at the top / in another section of the style guide. Swiftlint proposes to be a linter for the GitHub Swift style guide; we can use our fork as the Prolific Swift style guide linter, and that it would be included at the top of the style guide as the "official" linter for our style guide.

Thoughts @paulmiard @ThibaultKlein

@ppatriotis
Copy link

@Haud all for that if we had more of our rules in there besides the imports

@Haud
Copy link
Contributor Author

Haud commented Feb 24, 2016

@prolificphotis a bunch of our rules are already covered by the linter. I can try to add rules for stuff that the linter doesn't cover as we go forward.

@Haud
Copy link
Contributor Author

Haud commented Feb 26, 2016

@prolificphotis Automation covered in PR #34

@ppatriotis
Copy link

👍

@paulmiard
Copy link
Collaborator

Thought: should we use some sort of marker for the rules that are covered by our linter? (cf #34)

@paulmiard
Copy link
Collaborator

re:my last question: created an issue for that: #36

Haud added a commit that referenced this pull request Mar 11, 2016
Added ruleset for alphabetizing import statements
@Haud Haud merged commit c8eeddb into master Mar 11, 2016
@Haud Haud deleted the import_statements branch March 11, 2016 18:58
@paulmiard
Copy link
Collaborator

👍

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 this pull request may close these issues.

None yet

7 participants