Skip to main content

Contributing

👀 Review and Merge proceedings

This document explains the process of raising a Pull-Request with changes to the main Acter project and how it is dealt with.

General Rules

  • All code that changes the logic or has wide impact must be reviewed and approved by at least one other Contributor
    Reviews are an important part of our Quality Assurance process, every (code) contributor takes part in reviewing.
  • Smaller PRs are better than bigger ones
    The bigger a PR the more daunting it seems and the longer it stays around
  • Keep PRs primarily about one thing
    Minor related bug fixes on the way can be accepted but should be pointed out so people
  • Exceptions apply, see Special labels for more

Raising a PR

There's a few things to keep in mind when raising a PR that we'd like to point out.

Title and Description

Obviously you should pick a descriptive name and explain what the changes of the PR are, to set the context for the reviewer – We are not insisting on any particular pattern or format. You can also add additional notes for the reviewer either in the description or via leaving comments via github. It speeds up the process a lot if the reviewers know that a file doesn't actually contain any edits but that the code just split up for example.

Add a screencast

If your PR changes the design or adds a new feature, a screencast demoing the feature in action is always highly appreciated – from the code alone it can be hard to judge whether this renders well. If by addressing the feedback this significantly changes, you might want to update the main description with the fresh screencast or one that shows that particular change.

Add a changelog entry

Changes that impact the user experience in a meaningful way, let that be fixes or new feautures, should have a changelog entry added. Just created a new .md-file (we usually prefix this with the issue or PR number to avoid conflicts) in the .changes/-folder with a --prefixed list of items that changed. Keep in mind that this is meant for end-users to read and understand. Keep device-specific names (e.g. on Android) out of the text (refer to supported devices or alike) as we auto generate the weekly updates of that files and unrelated device indicators are rejected by some app stores.

💡
If you are using a branch on the main repo, a nice little bot will remind you in a comment to add your changelog entry in case you forgot to add one.

Tagging and asking for review

It is recommended to use the tags as appropriate, giving the reviewers even further indicators what the changes are about. There's also a few special tags around procedures or triggering certain CI features, you might want to familiarize yourself with.

🚀
Congrats, you've successfully raised your PR.

It's still your PR

It's not done until it is done. Keep your eyes close to the PR you've raised, it is still your responsibility to get it through. If, after days, no one reviewed it, make noise about in the appropriate channel or ping people, ask them what keeps them from reviewing it – you might want to split it into several parts.

Address the Feedback

When a review is made, address all the feedback. That doesn't always mean changing code, sometimes you just need to explain what is going as a comment to the specific comment – but all feedback must be addressed before moving forward. If the review was left in a changes requested-state, re-request a new feedback from that person after you've addressed the problems. If you aren't sure about what they addressed, reach out to them directly and discuss it in a DM.

Sometimes, it can also be agreed by author and reviewer that certain feedback isn't addressed directly but either through documenting it via a Github Issue or in a follow up PR to unblock this one being merged.

Keep an eye on CI

Our rule is that "main always builds & run" as we cut our weekly releases directly from that. Thus we have a pretty extensive CI process run before any PR can be merged. If any of the required CI checks fails, it is your responsibility to fix them.

Merging a PR

Once a PR has no more request changes-reviews, has at least one approval review and all feedback has been addressed (see above) and the CI passes, anyone with the permissions to merge it, can, including yourself. If you don't want that to happen just yet, mark the PR as draft again to avoid confusion.