Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
149 lines (109 loc) · 7.03 KB

CONTRIBUTING.md

File metadata and controls

149 lines (109 loc) · 7.03 KB

Engineering & Contributing Guidelines

The following is a set of guidelines for contributing to this repository. These are mostly guidelines, not rules. Use your best judgement, and feel free to propose changes to this document in a pull request. The processes described here is not to pester you but to increase and maintain code quality.

Working with this repository

We use issues to organise and prioritise work items.

Assignee meaning in issues: The assignee is the person responsible for following up on the issue (making sure it eventually gets addressed). It is usually (but not necessarily) the one working on it.

After picking up an issue, create a branch. There can be any number of branches and pull requests for one issue. But make sure that each issue is clearly linked to the pull request. There must be one pull request that closes the issue. If there are multiple PRs for an issue, make sure this is clear in the pull request.

Pull Requests

We use the GitHub-based PR workflow. When starting to work on an issue, create a branch and an according pull request that fixes the issue. The changeset in a pull request must not be larger than 1000 lines (with some exceptions for test snapshots or generated code). If an issue needs more work than that, split it into multiple pull requests.

After submitting the pull request, verify that all status checks are passing before asking for review.

While the prerequisites above must be satisfied prior to having your pull request reviewed, the reviewer(s) may ask you to complete additional design work, tests, or other changes before your pull request can be ultimately accepted.

PR & Commit Guidelines

  • Split out mass-changes or mechanical changes into a separate PR from the substantive changes.
  • Separate commits into conceptually-separate pieces for review purposes (even if you then later collapse them into a single changeset to merge), if technically possible.
  • Address all comments from previous reviews (either by fixing as requested, or explaining why you haven't) before requesting another review.
  • If your request only relates to part of the changes, say so clearly.

Force pushing

It is fine to force-push either (1) before asking for a review or (2) after PR approval, just before merging. Otherwise, in between two reviews, please do not force-push.

Regressions

When a PR introduces a regression, a fix should be submitted in a window of 2 days, otherwise the PR will be reverted.

Rules for the OCaml code

  • Never use the OCaml standard library, always use base, core or stdlib instead.
  • Avoid non-total functions (e.g. all the _exn functions in base).
  • Try to avoid exceptions, if possible.
  • Never use ==, which is the physical equality, and almost never what you want.

Changelog

Our changelog format is based on https://keepachangelog.com/. Please add an entry in a subsection (Added, Changed, Deprecated, Removed, Fixed -- see https://keepachangelog.com/en/1.0.0/#how) for each notable change.

Please prefix with engine:, frontend: or similar.

Should I add an entry to CHANGELOG.md?

Include in CHANGELOG.md:

  • New features and enhancements
  • Bug fixes
  • Breaking changes
  • Security patches
  • Major documentation updates
  • Dependency updates that affect users

Do not include:

  • Code refactoring with no user impact
  • Minor doc fixes (typos, grammar)
  • CI/CD or tooling changes with no external effect
  • Linting, formatting, or style-only commits
  • Reverts or fixup commits
  • Dependency bumps with no behavioral impact

Rule of thumb: If a user (developer or customer) wouldn’t notice or need to know, leave it out.

Styleguides

Optional Title Prefixes for Issues

To help quickly convey the focus of an issue, we sometimes add a short prefix in square brackets at the start of the title: [prefix] Issue short title. This is optional; you can just use it if the issue has a clear direction or goal.

Keep it short and intuitive, think of it as a lightweight hint, not a strict taxonomy or replacements for labels or milestones.

Use it when it helps. Leave it out when it doesn’t.

Git Commit Messages

  • Use the present tense
  • Use the imperative mood
  • Limit the first line to 80 characters
  • Don't end the first line of the commit message with a period
  • Reference issues and pull requests liberally after the first line
  • If the patch is of nontrivial size, point to the important comments in the non-first lines of the commit message.

Styleguide

Use rustfmt for every Rust code and ocamlformat for every OCaml code. From the command line, run cargo fmt in the root of hax and dune fmt in engine.

Documentation Styleguide

Use rustdoc comments on Rust files and functions. Use odoc comments on OCaml files. It is mandatory on public functions and encouraged on internal functions.

Reviews

As a reviewer always keep in mind the following principles

  • Reviewing code is more valuable than writing code as it results in higher overall project activity. If you find you can't write code any more due to prioritizing reviews over coding, let's talk.
  • You should respond to a review request within one working day of getting it, either with a review, a deadline by which you promise to do the review, or a polite refusal. If you think a patch is lower priority than your other work communicate that.

Review Guidelines

  • Check that the issue is assigned and linked.
  • Commit title and message make sense and says what is being changed.
  • Check that the PR applies cleanly on the target branch.
  • Check new files for license and administrative issues.
  • Check out code changes
    • Run automated tests
    • Manually verify changes if possible
  • Code review
    • Does the change address the issue at hand?
    • Is the code well documented?
    • Do you understand the code changes?
      • If not, add a comment. The PR can't be accepted in this stage.
    • Is the public API changed?
      • Are the changes well documented for consumers?
      • Do the changes break backwards compatibility?
      • Is the new API sensible/needed?
    • Is the code maintainable after these changes?
    • Are there any security issues with these changes?
    • Are all code changes tested?
    • Do the changes effect performance?
    • Look at the interdiff for second and subsequent reviews.
  • Ask if more information is needed to understand and judge the changes.

AI guidelines

Using AI tools to generate code for Hax is accepted under the following conditions:

  • The PR should clearly state that AI has been used and say for which parts of the code, tests, or documentation.
  • The author should also explain the methodology: how AI has been used and how the result has been tested.
  • Any AI generated content should be carefully reviewed by the author of the PR (before the reviewer).