|
| 1 | +# Contribution Guide for Learn-OCaml |
| 2 | + |
| 3 | +Thanks for considering contributing to the Learn-OCaml project! |
| 4 | + |
| 5 | +The guidelines below summarize the main conventions involved in the |
| 6 | +development of Learn-OCaml. |
| 7 | + |
| 8 | +## Issues |
| 9 | + |
| 10 | +Bug reports and feature requests are very welcome. They are tracked as |
| 11 | +GitHub issues, using various [labels](https://github.com/ocaml-sf/learn-ocaml/labels). |
| 12 | + |
| 13 | +First, search if a related issue already exists in the |
| 14 | +[`ocaml-sf/learn-ocaml` bug tracker](https://github.com/ocaml-sf/learn-ocaml/issues). |
| 15 | + |
| 16 | +Otherwise, you can open a new issue using one of the |
| 17 | +[issue forms](https://github.com/ocaml-sf/learn-ocaml/issues/new/choose). |
| 18 | + |
| 19 | +## Pull Requests |
| 20 | + |
| 21 | +In the sequel, we assume you are familiar with [Git](https://git-scm.com/docs/). |
| 22 | + |
| 23 | +We use pull requests to review bug fixes and new features. |
| 24 | + |
| 25 | +If the underlying bug or feature request has not been reported |
| 26 | +beforehand, it can be a good idea to open an issue first (unless it is |
| 27 | +a minor one). |
| 28 | + |
| 29 | +Then, you can state in this issue that you are working on a fix and/or |
| 30 | +discuss the design of the implementation with Learn-OCaml maintainers. |
| 31 | + |
| 32 | +Next, you may want to read the documentation regarding |
| 33 | +[How to set up your development environment](https://ocaml-sf.org/learn-ocaml/howto-setup-exercise-development-environment). |
| 34 | + |
| 35 | +### Branches Conventions |
| 36 | + |
| 37 | +Pull Requests should be created from a feature branch (≠ `master`), |
| 38 | +typically from a fork of |
| 39 | +[`ocaml-sf/learn-ocaml`](https://github.com/ocaml-sf/learn-ocaml), and |
| 40 | +target the `master` branch. |
| 41 | + |
| 42 | +If you need to fix merge conflicts, we generally prefer that you |
| 43 | +rebase your branch on |
| 44 | +[`master`](https://github.com/ocaml-sf/learn-ocaml/tree/master), |
| 45 | +rather than creating a merge commit. |
| 46 | + |
| 47 | +### Atomic Commits |
| 48 | + |
| 49 | +Borrowing some suggestions of the |
| 50 | +[Git Style Guide](https://github.com/agis/git-style-guide#commits), ideally: |
| 51 | + |
| 52 | +> * Each commit should be a single *logical change*. |
| 53 | +> Don't make several *logical changes* in one commit. |
| 54 | +> For example, if a patch fixes a bug and optimizes the performance of a feature, split it into two separate commits. |
| 55 | +> |
| 56 | +> * Don't split a single *logical change* into several commits. |
| 57 | +> For example, the implementation of a feature and the corresponding tests should be in the same commit. |
| 58 | +
|
| 59 | +Thus, the Learn-OCaml maintainers may suggest you adapt the commits of your PR |
| 60 | +(using [`git rebase -i`](https://git-scm.com/docs/git-rebase#Documentation/git-rebase.txt--i) and `git push -f`) |
| 61 | +to better comply with these suggestions as well as with the [Conventional Commits](#conventional-commits) guidelines below. |
| 62 | + |
| 63 | +However, these conventions can be somewhat lifted if the Pull Request |
| 64 | +is intended to be "squashed" in a single commit, as the maintainer |
| 65 | +could refine the squashed-commit message in this case (cf. the |
| 66 | +[Learn-OCaml maintainers wiki](https://github.com/ocaml-sf/learn-ocaml/wiki/Checklist-for-testing-and-merging-a-PR#merging-a-pr)). |
| 67 | + |
| 68 | +### Conventional Commits |
| 69 | + |
| 70 | +The commits messages should follow the [Conventional Commits specification](https://www.conventionalcommits.org/en/v1.0.0/). |
| 71 | + |
| 72 | +This is necessary as Learn-OCaml relies on the |
| 73 | +[`release-please`](https://github.com/googleapis/release-please) tool to |
| 74 | +automatically generate the [`CHANGELOG.md`](./CHANGELOG.md) and the |
| 75 | +[Release Notes](https://github.com/ocaml-sf/learn-ocaml/releases), |
| 76 | +assuming [Semantic Versioning](https://semver.org/). |
| 77 | + |
| 78 | +To sum up, each commit message contains a header, a body, and a footer, with the following structure: |
| 79 | + |
| 80 | +<pre> |
| 81 | +<i><b>type</b>[optional <b>scope</b>]</i><b>: <i>description</i></b> |
| 82 | +<i>blank_line</i> |
| 83 | +<i>[optional <b>body</b>]</i> |
| 84 | +<i>blank_line</i> |
| 85 | +<i>[optional <b>footers</b>]</i> |
| 86 | +</pre> |
| 87 | + |
| 88 | +where: |
| 89 | + |
| 90 | +* ***`type`*** is one of the <a href="#conventional-commits-types">conventional commits types</a> (`feat`, `fix`, …); an exclamation mark after the type denotes a non-backward-compatible change (e.g., <code>refactor<b>!</b>: Use ocamlformat</code>) |
| 91 | +* ***`scope`*** is a keyword *between parentheses* providing more context on the impacted part of the project (e.g.: API, grader, UI, dune, make, opam, docker, GHA, or just a given *filename.ml*) |
| 92 | +* ***`description`*** is a mandatory summary (typically starting with a verb in imperative, present tense), with no period in the end; it should be short but informative, just like an e-mail subject line. |
| 93 | +* ***`body`*** is an optional body, useful to explain *why* the change is necessary or has been implemented this way. |
| 94 | +* several kinds of ***`footers`*** can be provided: |
| 95 | + * References that auto-close issues → `Fix #1` or `Close #1` [(online doc)](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword) |
| 96 | + * References to commits or issues → `Related: #1` or mere URLs [(online doc)](https://docs.github.com/en/github/writing-on-github/working-with-advanced-formatting/autolinked-references-and-urls#issues-and-pull-requests) |
| 97 | + * A Conventional Commits footer → <code>BREAKING CHANGE: <i><b>description</b></i></code> |
| 98 | + (which plays the same role as the <code><b><i>type</i>!</b></code> suffix mentioned above) |
| 99 | + * A co-authorship footer → `Co-authored-by: First Last <[email protected]>` [(online doc) ](https://github.blog/2018-01-29-commit-together-with-co-authors/#how-it-works) |
| 100 | + * Or any footer following the usual [Git trailers convention](https://git-scm.com/docs/git-interpret-trailers). |
| 101 | + |
| 102 | +### Conventional Commits Examples |
| 103 | + |
| 104 | +Here are two example commit messages: |
| 105 | + |
| 106 | +* [`7e389ef`](https://github.com/ocaml-sf/learn-ocaml/commit/7e389ef22842e95e1b3f4364c19cf657a53ebf01) (commit obtained after squash-merging [PR #434](https://github.com/ocaml-sf/learn-ocaml/pull/434)) |
| 107 | + ``` |
| 108 | + feat(release.yml): Add a (3 jobs)-based GHA using release-please |
| 109 | + |
| 110 | + * Use var `OPAM_RELEASE` (GitHub PAC) |
| 111 | + |
| 112 | + * Use `expect` to workaround the fact that the feature wish |
| 113 | + https://github.com/ocaml-opam/opam-publish/issues/132 |
| 114 | + is not yet available. |
| 115 | + |
| 116 | + Co-Authored-By: Yann Régis-Gianas <[email protected]> |
| 117 | + Co-Authored-By: Erik Martin-Dorel <[email protected]> |
| 118 | + ``` |
| 119 | +* [`35941b5`](https://github.com/ocaml-sf/learn-ocaml/commit/35941b5ebe8cb2b947cd6010118050a79c6e36f8) (commit being part of [PR #448](https://github.com/ocaml-sf/learn-ocaml/pull/448)) |
| 120 | + ``` |
| 121 | + fix(grader): Display negative numbers with mandatory parens; Fix #440 |
| 122 | + |
| 123 | + * Thanks @letouzey for reporting this issue and suggesting a fix |
| 124 | + |
| 125 | + * Update some learn-ocaml-client tests accordingly |
| 126 | + ``` |
| 127 | + |
| 128 | +### Conventional Commits Types |
| 129 | + |
| 130 | +As specified in commit [`87bb9b5`](https://github.com/ocaml-sf/learn-ocaml/commit/87bb9b5e838badf872b8d08228e6768ce45710b5), the table below summarizes the commit types (lowercase prefixes before a colon) that are recognized by [the `release-please` GitHub Action](https://github.com/ocaml-sf/learn-ocaml/blob/master/.github/workflows/release.yml): |
| 131 | + |
| 132 | +| commit type | `CHANGELOG` section title | Comments | |
| 133 | +|-------------|---------------------------|----------| |
| 134 | +| `feat` | Features | Add a new feature (use `feat!` if it is non-backward compatible) | |
| 135 | +| `fix` | Bug Fixes | Patch a bug | |
| 136 | +| `revert` | Reverts | Revert commit (include that commit header, SHA1, and motivation) | |
| 137 | +| `perf` | Performance Improvements | Change code to improve performance | |
| 138 | +| `refactor` | Code Refactoring | Change code without adding a feature nor fixing a bug | |
| 139 | +| `deps` | Dependencies | Change external dependencies (e.g., for scopes opam, docker) | |
| 140 | +| `build` | Build System | Change the build system (e.g., for scopes dune, make, docker) | |
| 141 | +| `test` | Tests | Add missing tests or correct existing tests | |
| 142 | +| `ci` | CI/CD | Change the CI/CD configuration | |
| 143 | +| `docs` | Documentation | Change documentation only | |
| 144 | +| `style` | Style | (hidden by default) Change code without affecting its meaning (white-space, formatting, semi-colons or so) | |
| 145 | +| `chore` | Miscellaneous Chores | (hidden by default) Change files unrelated to code, tests, docs, build or ci config | |
| 146 | + |
| 147 | +See also: |
| 148 | + |
| 149 | +* [Conventional Commits v1.0.0 Summary](https://www.conventionalcommits.org/en/v1.0.0/#summary) |
| 150 | +* [Angular's Commit Message Format](https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit) |
| 151 | +* [Semantic Versioning](https://semver.org/) |
| 152 | + |
| 153 | +## Licensing |
| 154 | + |
| 155 | +Contributions to this repository are placed under the [MIT](https://spdx.org/licenses/MIT.html) license. |
| 156 | +This means that we can merge them with the same license as the rest of the codebase, while you keep all the rights on your code. |
| 157 | +And we will not have to bother you with any future license update. |
0 commit comments