Githawk: Project Management + Automation

[Huddie]

This is a bit of a big project but I think at the end we'll have a stronger foundation.
We want to do a few things to improve the repo on a project managment standpoint. This will allow (hopefully) a more standard process for pull request reviews + merging which will in turn allow for quicker mergers. More confident merging of PR's, a stronger more covered codebase etc.

Keep code clean (Unused code)

Periphery

As changes are made and code is changed, it can be hard to keep track of what code is no longer used. Sometimes even a new PR will introduce unused code simply because while creating the code you changed your approach and forgot to clean up. Periphery, suggested by @Sherlouk, seems to be a great project for detecting this (Now open sourced).

End goal

The hope is to incorperate this into Bitrise and Danger. Danger would hopefully post a comment when new unused code is introduced.

| Status |
| ------- |
| Discussion |

Keep code covered (Code Coverage)

We currently try and request users add unit tests with new (non-trivial) PR's. Our current contribution guide says that unit tests aren't mandatory due to wanting quicker releases. As that was written 2 years ago, I think times may have changed. Our code coverage currently is at 9% 🤕.

Suggestions to fix:

| Suggestions | Status |
| ------- | -------|
| Settle on a testing standard | Discussion |
| Create a testing guide | Discussion |
| Enforce testing for non-trivial additions | Discussion |
| Integrate code coverage reports into Bitrise | Discussion |

Testing Standard

• Swift Snapshot testing

Code Coverage

• Codecov-Bitrise Step
• Coveralls
• Slather

Keep code formatted (Linting/Formatting)

• SwiftFormatter
• SwiftLint

The difference here is between informing the user of formatting errors vs fixing them automatically. Both can be added to Danger/Bitrise for automation.

Keep code accessable (Accessability)

• The PR guide should require PR's to uphold a certain minimum level of accessability.
• If we can somehow automate with Danger a test for how accessable new code is that would be cool.

| Status |
| ------- |
| Discussion |

Keep Repo clean

• Maintainers will make sure to keep issues labeled (Dupe, feature request...etc)

• Request that no WIP PR's are created.

  • Reasoning: Typically these PR end up cluttering up the open PR list and are often dropped later on without being closed. PR's should be ready for review with the expectation of being merged as early as possible. If you'd like your work to be reviewed by others just link to your branch on your personal fork.

• Commenting on PR's are welcome but should be specifically about the PR. If its a future addition, the comment should be made into an Issue.

| Status |
| ------- |
| Discussion |

Standardize the Review Process

• 2 stages
  
  
  
  • Initial Review: PR is reviewed by project maintainers
  
  
  • Final Review: PR is reviewed by lead maintainer
  
  

To help with this new labels will be created:

• awaiting-review
• awaiting-final-review

• awaiting-minor-changes (No need for a basic review again, the lead maintainer will review once the minor changes are made)

• Request that no other members review the PR (Only the selected maintainers + lead maintainer)

  • Reasoning: Many people reviewing a PR tends to: 1) Clutter the PR and make navigation around and understanding its current state very confusing for everyone. 2) Different styles and suggestions (even conflicting ones) are sometimes made which leads to extra work for the PR creator and again, clutter + confusion.

| Status |
| ------- |
| Discussion |

[Huddie]

Obviously, all suggestions for changes, additions, removals, etc etc is welcome. This was just to get a small start on a big project. The idea of automating and smoothing out a bunch of processes has been a desire of a few maintainers so hopefully, this gets the ball rolling.

[wayni208]

FWIW, I’ve recently been using swiftformat instead of swiftlint autocorrect. It’s quite a bit faster and does a better job.

The new proposal for swift-format is supposed to be out of review now. Would it make sense to consider it as well?

[Huddie]

@wayni208 ya! I’ll add it. I also agree that swift lint is not as appealing as a formatter. Formatters allow
You to program how you want and the formatter takes care of everything else.

_{Sent with GitHawk}

[BasThomas]

Awesome stuff, @Huddie! Let me know if there's anything I can help with.

[Huddie]

@BasThomas I have it that all these “changes” are in discussion. I was honestly hoping for some feedback from various contributors. I didn’t think my initial suggestions would be the best and there are a few suggestions that have multiple options (Swift Format vs Swift Lint etc). The one thing I’d say is that if you can get Danger churning that would be a good start. I’ll try getting periphery up after that.

_{Sent with GitHawk}

[BasThomas]

Sorry; here you go!

Periphery: would love to see what we can do with it 👍
Enforce testing: will pick that one up
Linter: no preference
Accessibility: will pick that one up, at least adding a note to the contributing guidelines
WIP PR's: I'm OK not shying away from those now that we can use draft pull requests
Reviews: I don't think we should hinge reviews on one/a lead maintainer. We should be a bit more flexible there, using our judgement.

[flowinho]

Hey!
I highly suggest to also have a look at Travis-CI which offers extensive CI support for free as long as the project is OpenSource.
I've set up several iOS projects on GitHub using Travis and so far, they have been a great partner.
Maybe you'd like to have a look at it.
They also integrate quite nice with GitHub itself which means easy pull-requests based build-tests and easier user-management for the maintainers of the project.