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

Update code style to common Swift style #125

Merged
merged 7 commits into from
Apr 17, 2017
Merged

Update code style to common Swift style #125

merged 7 commits into from
Apr 17, 2017

Conversation

moogle19
Copy link
Contributor

Hi,
I updated some of the variable declarations with spaces after the colons and also omitted self's where possible (as recommended).

@k0nserv
Copy link
Contributor

k0nserv commented Apr 12, 2017

This is great 👍 I've been thinking about swiftlint recently and this looks like a good step in that direction. Feel free to add it to this PR if you feel inclined, otherwise I'll do it on the back of your work.

@k0nserv
Copy link
Contributor

k0nserv commented Apr 12, 2017

Also please credit yourself in CHANGELOG.md under the master section

Added swiftlint build phase and fixed errors and warnings generated by
swiftlint
@moogle19
Copy link
Contributor Author

@k0nserv
I added swiftlint as a Build Phase (not sure if should run automatically on build or manually?) and fixed the errors and warnings generated by swiftlint.

I used the default swiftlint settings (not sure if something specific should be enabled/disabled).

To fix the line length warnings (120 characters) I used this style of indentation.

addErrorButton?.setTitle(NSLocalizedString("Add error",
                                           tableName: "SkyFloatingLabelTextField",
                                           comment: "add error button title"), for: .normal)

I hope this is okay or is there a preferable style?

Let me know when something is wrong or needs change.

(There also was a commit I did not intend to push which I reverted afterwards)

@k0nserv
Copy link
Contributor

k0nserv commented Apr 14, 2017

I think I prefer this style

addErrorButton?.setTitle(
    NSLocalizedString("Add error",
    tableName: "SkyFloatingLabelTextField",
    comment: "add error button title"), for: .normal
)

@bogren opinions?

@bogren
Copy link
Contributor

bogren commented Apr 14, 2017

Or like this? 😃

addErrorButton?.setTitle(NSLocalizedString("Add error",
                                           tableName: "SkyFloatingLabelTextField",
                                           comment: "add error button title"),
                         for: .normal
)

I think I prefer @k0nserv suggestion but put the next parameter below as well :)

addErrorButton?.setTitle(
    NSLocalizedString("Add error",
                      tableName: "SkyFloatingLabelTextField",
                      comment: "add error button title"),
    for: .normal
)

@k0nserv
Copy link
Contributor

k0nserv commented Apr 14, 2017

This is how I would write it

addErrorButton?.setTitle(
    NSLocalizedString(
        "Add error",
        tableName: "SkyFloatingLabelTextField",
        comment: "add error button title"
    ),
    for: .normal
)

Copy link
Contributor

@k0nserv k0nserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, looks great :)

@k0nserv k0nserv merged commit e48d531 into Skyscanner:master Apr 17, 2017
@k0nserv
Copy link
Contributor

k0nserv commented Apr 17, 2017

Thanks a bunch for the PR @moogle19

@moogle19 moogle19 deleted the style_update branch April 17, 2017 10:27
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

3 participants