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

[Fastlane.swift] Add formatter to Fastlane.swift #16693

Merged

Conversation

minuscorp
Copy link
Collaborator

@minuscorp minuscorp commented Jun 28, 2020

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

I've seen that the Swift codebase doesn't follow any lint rules nor common syntax rules, so the first step is to integrate Swiftformat to ensure that every API or external change gets linted properly into the codebase. This not makes conflict to propose changes or cleanup code as proposed by another members.

Description

Testing Steps

@jhandguy
Copy link
Contributor

Awesome! 😍 Do you think it would make sense to add SwiftLint as well?

@minuscorp
Copy link
Collaborator Author

I just wanted a formatter, not a linter, if that difference makes sense. I just want to make sure that the code is following a set of common format rules and correct them if they fail (as Rubocop do) Swiftlint goes further with enabling and disabling rules and so on, I think it'll be too much for the project itself. What do you think?

@jhandguy
Copy link
Contributor

Of course, I was not questioning the choice of the formatter, formatters and linters do serve different purposes and both are valid in a project. I was only raising the possibility of adding SwiftLint on top of SwiftFormat, which could lint things that a formatter does not handle such as function and file length, nesting, etc.
And since some of the Fastlane Swift files require to be imported in the project (i.e. SnapshotHelper.swift), developers might expect those to be linted via SwiftLint.
But that's just a thought 👍

@minuscorp
Copy link
Collaborator Author

minuscorp commented Jun 29, 2020

The case is, the people will get the project already formatted into their Fastlane.swift, we do not force any rules because we do not embed the formatter into the project, just into the code that is being pushed. It is then user choice to make their own code changes, linting, etc. Any code that breaks is actually out of the organization, more of the user digging into the codebase 😛

EDIT: We let the user decide to lint the project or event add more format rules. We're just making babysteps to make the codebase more readable in different iterations 😄

@jhandguy
Copy link
Contributor

Makes sense @minuscorp, thanks for the clarification 👌

@minuscorp
Copy link
Collaborator Author

My plans are to improve the Swift current codebase as much as possible in further PRs, but I have a wait list 😪 it will come 😈

@minuscorp
Copy link
Collaborator Author

@joshdholtz 👀 😄

Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

This is 🔥 Never crossed my mind to format this lol. Thanks for adding this in 😍

@joshdholtz joshdholtz merged commit f3ed1bb into fastlane:master Jul 6, 2020
@fastlane-bot
Copy link

Hey @minuscorp 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉
The code change now lives in the master branch, however it wasn't released to RubyGems yet.
We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍
We'll notify you once we shipped a new release with your changes 🚀

Copy link

@fastlane-bot fastlane-bot left a comment

Choose a reason for hiding this comment

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

Congratulations! 🎉 This was released as part of fastlane 2.151.0 🚀

minuscorp added a commit to minuscorp/fastlane that referenced this pull request Jul 18, 2020
* Add linter to Fastlane.swift

* Formatting
@fastlane fastlane locked and limited conversation to collaborators Sep 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants