Skip to content

Latest commit

 

History

History
337 lines (286 loc) · 18.4 KB

File metadata and controls

337 lines (286 loc) · 18.4 KB

Contribution guidelines for Multipass

Rationale

Rowing a boat is more effective if everyone pulls in the same direction. The same idea applies to software projects. Shared goals, principles, and procedures result in less friction and lead to more consistent code, documentation, and user experiences.

At the same time, people have legitimate preferences, habits, and approaches that work well for them. They need a high degree of freedom to be creative and effective. Furthermore, the world changes and practices cannot all be anticipated at infinite resolution. It is thus clear that not every aspect of software engineering needs to be governed by pre-established rules.

It's good that we're humans, not machines: in comparison, we excel in adaptation. Too fine a rulebook would stifle the best we have to give, but we can still tune in on shared goals and protocols to streamline work, maximize harmony, and assimilate newcomers.

Accordingly, this document aims at establishing a set of guidelines that:

  • balance shared direction with autonomy;
  • match shared sentiment in the Multipass team;
  • afford a few grains of salt;
  • are meant to evolve.

Guidelines

In the lists that follow, prefixes are meant to provide an easy means of referring to individual guidelines and sections. Order may follow a loose line of reasoning, but it does not signify precedence or priority.

Meta-guidelines (META)

The following meta-guidelines cover the process to derive Multipass's contribution guidelines, along with governance goals.

META1. Everyone in the team can propose additional guidelines.
META2. Everyone in the team can question and propose changes to guidelines.
META3. Before the first set of guidelines is established, everyone in the team is invited to participate in live discussions about them.
META4. Before a new version of these guidelines is established, everyone in the team reviews it independently, except if away on prolonged absence.
META5. Ideally, all team members come to agree on any given version of these guidelines before it is established.
META6. Where that is not possible, preferably a majority of the team agrees with any given version of these guidelines before it is established.
META7. Preferably, all team members accept the latest established version of these guidelines, until the team agrees to modify it.
META8. In any case, all team members abide by the latest established version of these guidelines, until the team agrees to modify it.
META9. Established guidelines are taken seriously, but with a grain of salt. They are guidelines after all, not absolute rules.
META10. Once established, the first version of these guidelines is transferred to a versioned document (CONTRIBUTING.md).
META11. Going forward, these guidelines are meant to be expanded, refined, and corrected via pull requests.
META12. New practices can be experimented with before being adopted definitively.

Core principles (MU)

Principles for the members of the Multipass team. Many of these are inspired by Canonical's values, which we should keep in mind.

MU1. Aim at excellence.
MU2. Follow best practices (refer to other pertinent documents, e.g. CppCoreGuidelines).
MU3. Think critically (even about best practices).
MU4. Favor collaboration.
MU5. Be open to feedback.
MU6. Provide polite and sensitive feedback.
MU7. Indulge your reviewer (if it's all the same to the project and yourself).
MU8. If you believe you have it right, you have a right to defend why (arguably a duty).
MU9. When it comes to that, recognize that hierarchy determines who has the final word.
MU10. Be attentive and try to avoid mistakes.
MU11. Don't agonize over mistakes (you'll inevitably make them).
MU12. Be tolerant (nobody is perfect).
MU13. Strive to improve (you always can).
MU14. Leave the code a little bit better than you found it.
MU15. Aim for optimal solutions when they are feasible, compromise when they are not.

Releases (REL)

Descriptive rules of how releases are obtained from Git.

REL1. The trunk of Multipass development happens in the main branch, which releases branch out of.
REL2. Preferably, release branches contain only commits that are directly reachable from main or cherry-picked from it.
REL3. Cherry-picked commits in release branches may differ from the original ones only where necessary to avoid or fix conflicts.
REL4. In exceptional cases, release branches may contain dedicated commits for bug, build, or conflict fixes.
REL5. After a release is published, the corresponding release branch is merged back into main.

Pull Requests (PR)

Guidelines for how we use and handle pull requests.

PR1. Concrete modifications of Multipass can be proposed via Pull Requests (AKA PRs) targeting the main branch.
PR2. Prefer small, single-issue PRs.
PR3. A PR should introduce a coherent change that appears as a unit in a medium or high level of abstraction.
PR4. The main branch is modified exclusively via PRs, except for an empty commit after branching for release.
PR5. PRs accepted into main are merged with merge commits.
PR6. PRs to main should typically be covered by automated tests.
PR7. If a PR is valuable on its own, does not depend on others, and does not involve dead code, target the main branch, even if it is part of a larger task. This should be the most common case.
PR8. If your PR relies on another one, target the other's branch.
PR9. When PRs are stacked, prefer to merge them in order. The target branch will update automatically upon merging.
PR10. PRs should include descriptions and/or point to appropriate context (within reason).
PR11. When authoring a PR, make sure to test it.
PR12. When authoring a PR, make sure to review its diff.

Reviews (RVW)

RVW1. Multipass team members submit their PRs for review by assigning at least one reviewer: either a specific person of their choice or the "Multipass" team, for automated assignment.
RVW2. For PRs from external contributors, the Multipass team member on support duty at the time will delegate review assignment.
RVW3. Anyone in the Multipass team may request secondary reviews, including assigning themselves as a reviewer.
RVW4. Multipass team members may modify review assignments after discussion with the team.
RVW5. When assigning multiple reviewers, Multipass team members should indicate who is meant as primary and secondary reviewer.
RVW6. Reviewers should always request an additional secondary review if they're not fully confident in their understanding of all of the changes.
RVW7. The primary reviewer is responsible for closely inspecting the changes, considering the impacts, and appropriately testing the changes.
RVW8. Secondary reviewers are responsible for inspecting a particular aspect of the changes (e.g. architecture, UI design, domain-specific knowledge), to be defined by the person requesting the secondary review.
RVW9. After a PR is approved by multiple reviewers, small updates require only a single additional approval (i.e. after multiple approvals are dismissed).
RVW10. Review comments should be acknowledged by the author, but resolved by the reviewer.

Feature flags (FF)

FF1. When making an interdependent set of changes too large to review and merge as a single PR, consider adding a feature flag for it.
FF2. Feature flags should be used to disable any new code or behavior which is part of the given feature; with the feature flag disabled, Multipass should behave identically to before the flag's introduction.
FF3. Strive to encapsulate code that is dependent on a feature flag, preferably in its own files, so that those files can be entirely included or excluded from builds.
FF4. If necessary, create stub implementations of new feature APIs to allow other Multipass code to work with the new interfaces. When possible, put the stub implementations in separate files and conditionally compile them or the real implementations based on the state of the feature flag.
FF5. Where separate files are not feasible or justified, use preprocessor directives to selectively enable or disable feature code. Keep this approach as the exception rather than the norm.
FF6. Minimize unreachable code under a feature flag, i.e., code that can't be reached even though the feature is enabled.
FF7. PRs for code behind a feature flag should otherwise be treated as usual, with authors and reviewers maintaining the same standards of quality as for other PRs.
FF8. When a feature is fully-complete and suitable for release, the corresponding feature flag should be completely removed from Multipass.
FF9. Once a feature flag is removed, all newly-unreachable code should be removed along with it.

Versioning (GIT)

GIT1. Strive for atomic commits. A commit should introduce a coherent change that appears as a unit in a low level of abstraction.
GIT2. As a rule of thumb, commit messages of the form "do this and that" are an indication that there should be two commits instead.
GIT3. Strive to preserve a clean but detailed git history.
GIT4. Avoid squashing.
GIT5. Prefer additional commits during review (easier for reviewers to see the diff).
GIT6. Avoid merging the target branch back into the topic branch. Rebase instead.
GIT7. External contributors are encouraged to sign their commits, while Multipass team members are required to do so.
GIT8. Use kebab-case branch names (i.e. lower-case-words-separated-with-hyphens).
GIT9. Do not introduce whitespace errors.

Commit messages (MSG)

Guidelines for writing commit messages for non-merge commits. They are inspired by this and other posts. The category prefix is the main originality.

MSG1. Begin with a subject line.
MSG2. Start the subject line with a lower-case, single-word category, within square brackets (hyphenated, composite words are acceptable).
MSG3. If you find yourself wanting multiple categories, consider splitting commits. Otherwise, try to find a generic unifying category, or choose the most relevant.
MSG4. Leave a single space after the category and capitalize the first ensuing word.
MSG5. Limit the subject line to 50 characters (category included).
MSG6. Do not end the subject line with a period.
MSG7. Use the imperative mood in the subject line (e.g. "Fix bug" rather than "Fixed bug" or "Fixes bug").
MSG8. If adding a body, separate it from the subject with a blank line.
MSG9. Use multiple paragraphs in the body if needed. Separate them with a blank line.
MSG10. Do not include more than 1 consecutive blank line, except in quoted text.
MSG11. Use punctuation normally in the body.
MSG12. Wrap the body at 72 characters, except on lines consisting only of blockquotes, references, sign-offs, and co-authors.
MSG13. Use the body to explain what and why, rather than how.
MSG14. Be descriptive but succinct and avoid filler text.
MSG15. Omit the body if the subject is self-explanatory.
MSG16. Common abbreviations are fine (e.g. "msg" or "var").

Examples

  • Common categories: [daemon], [cli], [settings], [build], [ci], [test], [doc], [gui], [format]
  • Subject line: [gui] Fix button alignment in navigation bar
  • Full commit message with a body:
[completion] Avoid evaluating function input

Avoid calling `eval` with function arguments, to reduce the chance for
code injection (now or in the future).

Helper tools

  • Consider setting your commit template to the .gitmessage file in the repository root:
    • git config --local commit.template .gitmessage
  • Consider adding the commit-msg file in the repository root as a git hook (in .git/hooks)
    • ln -s ../../git-hooks/commit-msg.py .git/hooks/commit-msg

Dependencies (DEP)

DEP1. Acceptable mechanisms to adopt source-code dependencies are, in decreasing order of preference: Vcpkg (for C++) > FetchContent > submodule.
DEP2. Avoid vendoring (copied source code).

Code

Prescriptive programming and engineering principles to aim at. Many of these overlap. Some are in tension and have to be balanced.

Generic engineering and code (COD)

COD1. Avoid duplicate sources of truth.
COD2. Prefer small units, be they functions, classes, files, etc.
COD3. Follow SOLID principles.
COD4. Pay special attention to the principles of single responsibility and separation of concerns.
COD5. Don't repeat yourself (DRY).
COD6. Don't reinvent the wheel.
COD7. Don't overengineer (YAGNI).
COD8. Avoid coupling functionality that isn't logically coupled.
COD9. Encapsulate, within each unit, all information that other units don't need.
COD10. Encapsulate data with dependent behavior.
COD11. Strive for interfaces that are easy to use, but hard to misuse.
COD12. Avoid redundant levels of indirection.
COD13. Simplify. Avoid pointless complexity.
COD14. All else being equal, less code is better code.
COD15. Don't try to maximize LoC metrics.
COD16. Aim for consistency.
COD17. If it is all the same otherwise, follow a single approach (see how it is done elsewhere).
COD18. Be creative. If you have a better approach, propose it and discuss it openly.
COD19. Prioritize. Balance idealism with pragmatism.
COD20. Document public, user-facing interfaces.
COD21. Keep documentation up to date.

C++ (CPP)

CPP1. Stick to standard C++20, with the exception of #pragma once.
CPP2. Prefer #pragma once to header guards
CPP3. Prefer enforcing correct usage (prevent misuse) at compilation time.
CPP4. If a type isn't meant to be copied or moved, inherit from DisabledCopyMove (either directly or indirectly).
CPP5. For such types, define only constructors that fully initialize objects. Avoid a default constructor unless the object needs no parameterization.
CPP6. Make copyable types semiregular.
CPP7. Avoid two-stage initialization. Initialize objects fully in the constructor.
CPP8. Avoid const by-value params (e.g. no void foo(const bool flag);)
CPP9. Encapsulate platform-dependent functionality in dedicated units (types, functions) and do not use platform #ifdefs (or other platform-conditional logic) outside of those units.
CPP10. Use CamelCase for types, but snake_case for variables and functions.
CPP11. Avoid magic numbers.
CPP12. Declare generic constants in a dedicated header.
CPP13. Avoid compilation warnings.
CPP14. To mock free functions and external APIs, wrap them with MockableSingleton.

Text

Prescriptive guidelines concerning text and written communication, including but not limited to: PR descriptions, commit messages, comments, and documentation.

TXT1. Be clear, precise, informative, and organized.
TXT2. Avoid filler text or fluff.
TXT3. Avoid repetition.
TXT4. Use references, quotes, citations, or links to provide context, substantiate claims, and increase reliability.
TXT5. Express the same idea more than once only to add relevant information, improve precision, provide a different perspective, or help clarify complexity.

AI

Prescriptive guidelines concerning the use of Artificial Intelligence (AI) when contributing to Multipass, in particular generative AI and LLMs.

AI1. Every contribution must be authored by human beings (one or more), possibly with the help of AI tools. In addition to code contributions, this applies to issues, pull requests, discussions, and any other comments or communications, regardless of the platform.
AI2. Contributors are free to use any tools they see fit, including AI tools, provided they do so lawfully and ethically.
AI3. Git commits must have a human author, with a valid human-managed email address.
AI4. The author(s) of a contribution must be able to explain the contribution in detail, including any code or textual changes.
AI5. Contributors must refrain from submitting code, text, or any other content that they don't fully understand.
AI6. Contributors must submit only modifications that they have reviewed thoroughly, especially if it includes AI-generated content.
AI7. When using AI, contributors should make an extra effort to ensure good information density (i.e. high signal-to-noise ratio). This includes code and text.
AI8. Authors are ultimately responsible for their entire contributions, including all components, contents, format, presentation, and communication.

Further Information