Enforce SwiftFormat across the codebase

tsheaff commented 2 years ago

Rationale

Formatters are an excellent way to support developer velocity as more contributors are active in a codebase. Some advantages:

Avoid parts of the code feeling super different style-wise from other parts, giving the whole codebase a consistent feel, which drastically improves readability
Avoid bikeshedding about low-level style questions, leaving PR comments and dev time spent on adding user value
Automatically fix certain types of common mistakes

Why SwiftFormat?

The most full-featured and well-adopted swift formatter is SwiftFormat. It's very configurable (see rules here) but comes with a nice set of defaults. I've used it in other projects I've worked on, and everyone seemed to really like it.

Approach

This PR does the following:

Set up SwiftFormat in the codebase using Mint as the package manager
Run swiftformat . to enforce the style across the codebase. This is where ~99% of this diff comes from, and was all done automatically with that one command.
Enforce that SwiftFormat always pass before merging to master using GitHub Workflows. Once this PR merges, this can be enforced on the master branch by a repo admin under Settings > Branches, guaranteeing that everything on master always formats properly
Add installation instructions for SwiftFormat in the README.md so that devs can run it locally.

I'm super open to tweaking the .swiftformat configuration if the team feels some of the defaults or the few overrides I proposed aren't ergonomic for this codebase.

Rollout

If there are any outstanding branches that have not yet merged before this does, the process to avoid conflicts is very simple:

Merge master back to your branch after this merges.
If there are any conflicts, checkout ours to take your changes
Simply rerun swiftformat . locally to get the formatter passing once again.
When ready, PR your branch back to master

eclectocrat commented 2 years ago

Just want to comment: heck yes! I love code formatters, they remove stress and make things much easier to read and maintain. 👍

tzarebczan commented 2 years ago

Hey @tsheaff, thanks so much for working on this awesome improvement, and congrats on the first PR!

We'll get a review on it soon.

Can we show you some appreciation?

tsheaff commented 2 years ago

Thanks @tzarebczan I just sent an email for any appreciation 🎩

Once this merges, a repo admin should make the formatter a blocking step for merges on the master branch.

Any thoughts on the particular formatter rules @tzarebczan or @eclectocrat ? I think it looks good but y'all have (much) more experience with this codebase.

akinwale commented 2 years ago

This is great stuff, thanks! It's always nice to see developments like this. I will review this in a bit.

tsheaff commented 2 years ago

Thanks @akinwale !

@akinwale can you enforce the lint job to be a required step before merging under the repo's Settings > Branches > master settings?

OdyseeTeam / odysee-ios