Contributing to iOS

Code standards

Code architecture, principles and standards can be found on CODING_PRACTICES

Code Style

We are following Ray Wenderlich's Swift Style Guide.

Code editor

The prefered code editor is Xcode. Xcode comes with a good default configuration also this repository contains all required configuration files automatically loaded by the IDE.

Commit styling

Use the present tense and imperative mood when describing your commits:

• Instead of "Adding support for Swift", write "Add support for Swift".
• Instead of "Fixed brand profile view", write "Fix brand profile view".

This form is akin to giving commands to the code base and is recommended by the Git SCM developers.

Git workflow

The prefered git workflow is based on GitFlow but using master branch as target for the feature branches (as it was develop).

Branches & Naming

• master: Master is the main branch and contains code that has passed tests, QA verification and it's ready to be released.
• feature/ABC/ABC-XXX-your-feature-title: Feature branches fork from master branch and contain work that matches directly with a task/user story on Jira.
• subtask/ABC/ABC-XXX-your-subtask-title: Subtask branches fork from feature branches and contain small units of work necessary to accomplish the feature.
• hotfix/ABC/ABC-XXX-your-hotfix-title: Hotfix branches fork from release branch and aim to solve bugs on the current release.
• release/X.X.X: Release branches fork from master and contain code ready to go to production.

Pull Requests

Before submitting & recommendations

• Avoid merge conflicts: make sure your branch is rebased on the master branch of this repository.

• The PULL_REQUEST_TEMPLATE.md contains a checklist of tasks to do before asking for review.

• Add a link to Jira in the PR description (not on title): Since Github Jira Plugin is installed adding: [ABC-123] on the description will automatically translate to a Jira link.

• Clean up your commit history. Each commit should be a single complete change. This discipline is important to ease the work for the reviewers. It's always better to have something like this:

    * Add presenter A
    * Add module X
    * Add new data repo
    * Rm old data repo
  

  Instead of this:

    * add presenter A
    * fix presenter
    * add tests
    * fix tests
    * fix more tests
    * add module X !wip
  

• (Try to) Avoid PRs with too many changes: A large PR not only stretches the review time, but also makes it much harder to spot issues. In such case, it's better to split the PR to multiple smaller ones. For large features, try to approach it in an incremental way, so that each PR won't be too big. Also, try to create new commits and don't rewrite/squash the commit history. This way it's very easy for the reviewers to see diff between iterations. If you rewrite the history in the pull request, review could be much slower.

• Add a meaningful title on the PR describing what change you want to check in: Don't simply write: "Fix Jira issue #5". Also don't directly use the issue title as the PR title. An issue title is to briefly describe what is wrong, while a PR title is to briefly describe what is changed. A better example is: "Add Ensure parameter to New-Item cmdlet", with "Fix Jira #5" in the PR's body.

• Include a summary about your changes in the PR description: The description is used to create change logs, so try to have the first sentence explain the benefit to end users.

• Add your PR to your squad Github project (if any) in order to give visibility.

Work in Progress

If your pull request is not ready to merge nor review use a Draft Pull Request.

In case your PR goes from Ready to review back to Draft/WIP add the prefix WIP:, [WIP:] or 🚧 to the beginning of the title and remove the prefix when the PR is ready (Github does not allow to go back to draft). Also, remove the reviewers until the PR is ready or in exceptional cases (commit doubts etc.). Optionally use the WIP label to give even more visibility.

Automatic Checks

• After submitting your pull request, the CI service will run a suite of tests and automatically update the status of the pull request

Merging strategy

This repository only allows squash merges. The aim of this policy is to have a cleaner commit history both for master and for the feature branches.

On master, every squash commit MUST follow gitchangelog convention. This way the changelog is easier and auto generated on the release proccess, also include the Jira ticket number, eg: new: [ABC-1234] Create Signup.

Remember to remove the aggregated commit history in the description of the squash commit, otherwise it will appear in the CHANGELOG later.

Labels

Use the following labels to give more information about your PR:

• QA: The PR is being tested by QAs before merging to master. This is usually set when the PR is already approved.
• Blocked: The PR can't be merged to master for some important issue.
• Hotfix: The PR is a hotfix of current App Store version and needs to be prioritized.
• Release: The PR is a feature that will be included on next release and needs to be prioritized.
• Backlog: The Issue is added to our Tech Backlog.

Roles and Responsibilities

1. Author creates the pull request
2. Assignee is responsible for moving the PR forward to get it Approved. This includes addressing feedback within a timely period and indicating feedback has been addressed by adding a comment and mentioning the specific reviewers. Once the PR has been approved and the CI system is passing, the assignee will merge the PR.
3. Reviewers are CODEOWNERS (if any) and anyone who wants to contribute. They are responsible for ensuring the code: addresses the issue being fixed, does not create new issues (functional, performance, reliability, or security), and implements proper design. Reviewers should use the Review changes drop down to indicate they are done with their review.
   • Request changes if you believe the PR merge should be blocked if your feedback is not addressed,
   • Approve if you believe your feedback has been addressed or the code is fine as-is, it is customary (although not required) to leave a simple "Looks good to me" (or "LGTM") as the comment for approval.
   • Comment if you are making suggestions that the assignee does not have to accept. Early in the review, it is acceptable to provide feedback on coding formatting but after the PR has been approved, it is generally not recommended to focus on formatting issues unless they go against the most basic rules (missing new lines, bad indentation etc. ). Non-critical late feedback (after PR has been approved) can be submitted as a new issue or new pull request from the reviewer.

Workflow

1. The PR author creates a draft PR from a branch.
2. The author ensures that their PR passes the build and changes its state to Ready for review. Otherwise, the author SHOULD continue to update the pull request until the build passes.
3. The author MUST set an assignee: in most of cases would be himself but can be another contributor. The assignee is the one who is going to merge the PR.
4. The author adds his teammate to the list of reviewers. Other reviewers will be automatically added by Pull Panda.
5. Once the code review is completed, the assignee merges the pull request and ensures the correctness of the resulting merge.
6. (case feature PR) Execute script make qa with your JIRA ticket as a parameter ABC-XXX (e.g. make qa issue=LP-1234)

Abandoned PRs

A pull request with no changes for more than two weeks without a word from the author/assignee is considered abandoned and will be deleted (jointly with the branch) periodically.

Releasing

Read RELEASE.md for more information about creating a release.

New Ideas

Go to Issues on GitHub.

1. Create a feature request (a template is already in place).
2. Fill all the fields of the template and assign as many member as needed.
3. If there is an agreement, Chapter Lead will contact your Engineer Manager to add a task in your backlog.
4. If it is a huge task, we can always add it to the Tech Backlog and prioritise it for next Quarter.