Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
88 lines (53 loc) · 6.27 KB

CONTRIBUTING.md

File metadata and controls

88 lines (53 loc) · 6.27 KB


Contributing

We welcome suggested improvements and bug fixes for sentry-cocoa, in the form of pull requests. To get early feedback, we recommend opening up a draft PR. Please follow our official Commit Guidelines and also prefix the title of your PR according to the Commit Guidelines. The guide below will help you get started, but if you have further questions, please feel free to reach out on Discord.

For more detailed information, see develop-docs/README.md.

Environment

Run make init to get started. This will install pre-commit, bundler and Homebrew and their managed dependencies (see Gemfile and Brewfile).

Please use Sentry.xcworkspace as the entry point when opening the project in Xcode. It also contains all samples for different environments.

PR reviews

PR Status and Readiness

  • Pull requests which are not ready for review should stay in draft mode. Use GitHub's draft PR feature to indicate that your PR is still work in progress and not yet ready for maintainer review.
  • Pull requests which are ready to review should be marked as such. When your PR is ready for review, mark it as ready by converting it from draft to a regular PR.

Review Types and Meanings

For feedback in PRs, we use the LOGAF scale to specify how important a comment is. You only need one approval from a maintainer to be able to merge. For some PRs, asking specific or multiple people for review might be adequate.

Understanding review feedback:

  • PR comments are a request for change. When a reviewer leaves comments on your PR, they are requesting changes or asking questions that need to be addressed.
  • PR approvals are a confirmation it can be merged. An approval indicates the reviewer has reviewed your PR and confirms it's ready to be merged.

Our different types of reviews:

  1. LGTM without any comments. You can merge immediately.
  2. LGTM with low and medium comments. The reviewer trusts you to resolve these comments yourself, and you don't need to wait for another approval.
  3. Only comments. You must address all the comments and need another review until you merge.
  4. Request changes. Only use if something critical is in the PR that absolutely must be addressed. We usually use h comments for that. When someone requests changes, the same person must approve the changes to allow merging. Use this sparingly.

After Addressing PR Feedback

After addressing PR feedback, request another PR review via GitHub. This changes the pull request status back from commented/approved to waiting, ensuring maintainers are notified that you've addressed their feedback and the PR is ready for re-review.

PR Labels

ready-for-merge Label

The ready-for-merge label is used to enable CI of the pull request. It should be set as soon as the PR is ready to be tested in CI, not before, to save CI resources. Only add this label when your PR is actually ready for full CI testing.

Waiting for: Review :hourglass: Label

If a PR has been pending review for a while (typically several days), contributors can set the label Waiting for: Review :hourglass:, which will result in an internal reminder to maintainers.

Important: Contributors should not put the "Waiting for: Review ⌛" label on every PR, as we still have normal review processes where maintainers regularly look at open PRs. This label should only be used if PRs have not been reviewed for days and need additional attention.

Code Formatting

Please follow the convention of removing the copyright code comments at the top of files. We only keep them inside SentryCrash, as the code is based on KSCrash.

All Objective-C, C and C++ needs to be formatted with Clang Format. The configuration can be found in .clang-format. Formatting should happen automatically as part of our precommit hook, which uses make format.

GH actions suddenly formats code differently

It can be that it uses a different clang-format version, than you local computer. Please run make init, and ensure that your version (run clang-format --version) matches the one from GH actions. There is now a precommit step that ensures that the versions are synchronized, see scripts/check-tooling-versions.sh.

More information: We always use the latest version of clang-format in homebrew in our GH actions for formatting the code. As we use homebrew for setting up the development environment, homebrew only contains formulas for clang-format 8, 11, or the latest, and we want to use the latest clang-format version; we accept that we don't pin clang-format to a specific version. Using the GH action images clang-format version doesn't work, as it can be different than the one from homebrew. This means if homebrew updates the formula for the default clang-format version so does our GH actions job. If the GH actions job suddenly starts to format code differently than your local make format, please compare your clang-format version with the GH actions jobs version.

Linting

We use Swiftlint and Clang-Format. For SwiftLint, we keep a multiple config files for the tests and samples, cause some rules don't make sense for testing and sample code. To run all the linters locally execute:

make lint

Final Notes

When contributing to the codebase, please make note of the following:

  • Non-trivial PRs will not be accepted without tests (see above).
  • Please do not bump version numbers yourself.