DocumentationAPI ReferenceCommunity
Documentation

Developer Contribution Guide

Learn how to commit new code to Diffgram Open Core

Suggest Edits

Below we describe the contribution process for the code in Diffgram. The main purpose of describing this process is so:

  1. Contributors know what to expect from the maintainers (Possible comments, welcoming people, positive vibes, etc...)
  2. Maintainers know what to expect from new contributors (Using the latest version, address and describe the new changes correctly, tests, etc..)

Merging Pull Requests

For merging a Pull request, the PR must pass all the criteria of our Definition of Done. This means that we will not consider the PR finished until all of the following criteria's are passing the Definitions of Done.

See our Pull Request Template here: https://github.com/diffgram/diffgram/pull/1111

Once the definition of Done is complete the pull request may be merged by one of the project's maintainers.

Occasionally the core team may override this criteria and expressly approve or reject a pull request.

Definition of Done - Basics

  1. Clear description of the contribution: Describe the fix/feature you are adding in the PR. Any assumptions, purposes or related systems that the code will affect. We especially want to see the high level forest and context. If possible link to other documents, and include code samples, images or videos.

If the item in question is controversial for any reason we may ask you to provide support for why the proposed path is acceptable at this moment in time. For example if the pull request adds a function that may only be used by a small subset of users, but may triple maintenance efforts on another sub system it will need more justification then a clear bug fix of an existing problem.

Does it solve the problem? Have you tested it from the UI/SDK/API? Have you tested it with appropriate data?

  1. Writing clean code.
  • In general there should not be a need for comments because the code is well enough written. If appropriate scope, you have linked to a design document.
  • When needed you have made clarifying comments.
  • Naming. Variable, function, class, etc. names must be the best you can possibly come up with. If there's one thing to get as right as possible - it's the names. If you call something manager or controller or name a type list object the_id we will grill you!

📘

Requests must improve the existing system

In general, any moderate sized request should always refactor an existing bad part of the system. All code introduces future bad parts, so we must always be fighting this entropy. Only emergency type patches, or code segments that are clearly part of a new developing direction may be allowed to break this rule.

  1. Write the related unit/e2e tests.
    These must all pass on the CI system.

Definition of Done - System Integrity

  1. The new addition does not break any existing tests on our CI system.

👍

Must Pass all Existing CI

You can edit existing tests if they are no longer relevant. But it must eventually pass all.

All testing concepts have a degree of flakiness. Occasionally something may pass in a branch and then fail when merged to master. Occasionally if a test passes locally we will allow a merge to CI before remote passes. But barring these exceptions this is one of the most inflexible items on the list. The tests must pass.

  1. The code maintains the performance guidelines
  2. The code maintains the security guidelines.

Definition of Done - Documentation

  1. All required documentation is provided via a link or in a readme.md file inside the commit.
  2. Changelog entry added if necessary.

Definition of Done - Well Reviewed

  1. Reviewed and approved by all the relevant reviewers of the Diffgram Core Team.

👍

Fixed right the first time

There is a high level of expectation that once a small reviewed item is brought to attention, that it will be fixed appropriately by the contributor with minimal or zero further review by the reviewer. If there is any significant room for doubt, always ask first before submitting the revised code, or be clear in the submission that you want to discuss it further. This is critical to good communication and building mutual trust.

Definition of Done - Migration Aware

  1. Creating an Issue in the Repo with the infrastructure_change tag, if the changes add a new configuration that is relevant for future deployments at the infrastructure level.
  2. Creating an issue with the the db_schema tag on a new issue if there are changes at the DB level.

Definition of Done - Communication

  1. Added a release blog post, if relevant.
  2. Added to the main website, if relevant.

Communication Assumptions - Don't Rely on Github Notifications

Over communicate rather than under communicate.

  1. If it’s something active or time sensitive. send the link or heads-up message through another channel, eg slack or personal email. (We all get many git notifications.)
  2. @mention helps but just becuase you @mention someone don't assume they are guaranteed to look at.
  3. Consider the person may eventually see all the updates but don’t expect them to.

Crosslink often

  1. If a pull request is solving an issue, post in the Issue linking the pull request. That avoids someone who is focused on the issue thinking there are no updates (and potentially doing duplicate work.).
  2. Don’t assume it’s clear even if the pull request has a similar name etc because it won’t show up to someone who’s focused on that specific Issue.

Security Issues

Do not create a public issue. Email [email protected] with the details.

Assigning Issues

If an issue is urgent or really complex and need the attention of a specific person related with the code, you can assign the person to the issue. We don't want to discourage people from contributing if the issue is already assigned to a member, so feel free to add contributions even if the issue already has an assigned member.

Be kind and respectful

Be kind and respectful with all the people that are trying to contribute. Be aware that sometimes contributors may not be native English speaker or may not have the deep understanding of the code you may have. Use emojis to express feelings, also feel free to read our Code Review guidelines and our Code of Conduct.

Bugs

A bug is a defect or error in the system which failed to fulfill some product requirement. The severity of a bug can be from blocking an entire funcion or feauture in the system, to a simple usability bug. Please make sure to describe the severity when creating the bug issue, and to review if the bug is not a known bug before creating the issue. If the bug already exists, feel free to comment adding any relevant information in the comment section of the Github issue.

Copy and Paste Responses for Issues

Improperly Formatted Issues

Thanks for the issue report. Please reformat your issue to conform to the
[contributing guidelines](https://diffgram.readme.io/docs/contributing-guide).

Issue report for old version

Thanks for the issue report but we only support issues for the latest stable version of Diffgram Open Core.
I'm closing this issue but if you still experience this problem in the latest stable version,
please open a new issue (but also reference the old issue(s)).
Make sure to also include the necessary debugging information conforming to the issue tracker
guidelines found in our [contributing guidelines](https://docs.gitlab.com/ee/development/contributing/issue_workflow.html#issue-tracker-guidelines).

Merging

  1. Always merge the master branch to the PR before merging the pull request
  2. Always pull/refresh before branching from master eg to avoid missing things

Updated about 2 years ago