Skip to content

Latest commit

 

History

History
117 lines (86 loc) · 5.04 KB

CONTRIBUTING.md

File metadata and controls

117 lines (86 loc) · 5.04 KB
Raw

This is the contributing guide for DeGitX project. It could be updated at any time, esepcially when some contributing style inconsistency occures.

How to contribute

Fork the repository, make changes, and send us a pull request (PR). We will review your changes and apply them to the master branch shortly, provided they don't violate our quality standards.

To avoid frustration, before sending us your pull request please run full build:

make all lint

Before submitting pull-request please rebase to latest master branch and refresh dependencies:

make tidy

After submitting pull-request check CI status checks. If any check with "required" label fails, pull-request will not be merged.

Code style

Code style is enforced by golangci-lint tool. To run it use make lint.

Pull request style

Primary PR rule: it's the responsibility of PR author to bring the changes to the master branch.

Other important mandatory rule - it should refer to some ticket. The only exception is a minor type fix in documentation.

Pull request should consist of two mandatory parts:

  • "Title" - says what is the change, it should be one small and full enough sentence with only necessary information
  • "Description" - says how this pull request fixes a problem or implements a new feature

Title

Title should be as small as possible but provide full enough information to understand what was done (not a process), and where from this sentence. It could be capitalized If needed, started from capital letter, and should not include links or references (including tickets numbers).

Good PR titles examples:

  • Fixed Maven artifact upload - describes what was done: fixed, the what was the fixed: artifact upload, and where: Maven
  • Implemented GET blobs API for Docker - done: implemented, what: GET blobs API, where: Docker
  • Added integration test for Maven deploy - done: added, what: integration test for deploy, where: Maven

Bad PR titles:

  • Fixed NPE - not clear WHAT was the problem, and where; good title could be: "Fixed NPE on Maven artifact download"
  • Added more tests - too vague; good: "Added unit tests for Foo and Bar classes"
  • Implementing Docker registry - the process, not the result; good: "Implemented cache layer for Docker proxy"

Description

Description starts with a ticket number prefixed with one of these keywords: (Fixed, Closes, For, Part of), then a hyphen, and a description of the changes. Changes description provides information about how the problem from title was fixed. It should be a short summary of all changes to increase readability of changes before looking to code, and provide some context. The format is (<keyword>For|Closes|Fixes|Part of) #(<ticket>\d+) - (<details>.+), e.g.: For #123 - check if the file exists before accessing it and return 404 code if doesn't.

Good description describes the solution provided and may have technical details, it isn't just a copy of the title. Examples of good descriptions:

  • Added a new class as storage implementation over S3 blob-storage, implemented value() method, throw exceptions on other methods, created unit test for value
  • Fixed FileNotFoundException on reading blob content by checking if file exists before reading it. Return 404 code if doesn't exist

Merging

We merge PR only if all required CI checks passed and after approval of repository maintainers. We merge using squash merge, where commit messages consists of two parts:

<PR title>

<PR description>
PR: <PR number>

GitHub automatically inserts title and description as commit messages, the only manual work is a PR number.

Review

It's recommended to request review from @artipie/contributors if possible. When the reviewers starts the review it should assign the PR to themselves, when the review is completed and some changes are requested, then it should be assigned back to the author. On approve: if reviewer and repository maintainer are two different persons, then the PR should be assigned to maintainer, and maintainer can merge it or ask for more comments.

The workflow:

<required> (optional)
        PR created |   Review   | Request changes | Fixed changes | Approves changes | Merge |
assignee: <none>  -> <reviewer> ->    (author)    ->  (reviewer)  ->   <maintainer>  -> <none>

When addressing review changes, two possible strategies could be used:

  • git commit --ammend + git push --force - in case of changes are minor or obvious, both sides agree
  • new commit - in case if author wants to describe review changes and keep it for history, e.g. if author doesn't agree with reviewer or maintainer, he|she may want to point that this changes was asked by a reviewer. This commit is not going to the master branch, but it will be linked into PR history.

Commit style

Commit styles are similar to PR, PR could be created from commit message: first line goes to the title, other lines to description:

Commit title - same as PR title

For #123 - description of the commit goes
to PR description. It could be multiline `and` include
*markdown* formatting.