15 KiB

Raw Blame History

Contributing to Klipper

Thank you for contributing to Klipper! This document describes the process for contributing changes to Klipper.

Please see the contact page for information on reporting an issue or for details on contacting the developers.

Overview of Contribution Process

Contributions to Klipper generally follow a high-level process:

A submitter starts by creating a GitHub Pull Request when a submission is ready for widespread deployment.
When a reviewer is available to review the submission, they will assign themselves to the Pull Request on GitHub. The goal of the review is to look for defects and to check that the submission follows documented guidelines.
After a successful review, the reviewer will "approve the review" on GitHub and a maintainer will commit the change to the Klipper master branch.

When working on enhancements, consider starting (or contributing to) a topic on Klipper Discourse. An ongoing discussion on the forum can improve visibility of development work and may attract others interested in testing new work.

What to expect in a review

Contributions to Klipper are reviewed before merging. The primary goal of the review process is to check for defects and to check that the submission follows guidelines specified in the Klipper documentation.

It is understood that there are many ways to accomplish a task; it is not the intent of the review to discuss the "best" implementation. Where possible, review discussions focused on facts and measurements are preferable.

The majority of submissions will result in feedback from a review. Be prepared to obtain feedback, provide further details, and to update the submission if needed.

Common things a reviewer will look for:

Is the submission free of defects and is it ready to be widely deployed?

Submitters are expected to test their changes prior to submission. The reviewers look for errors, but they don't, in general, test submissions. An accepted submission is often deployed to thousands of printers within a few weeks of acceptance. Quality of submissions is therefore considered a priority.

The main Klipper3d/klipper GitHub repository does not accept experimental work. Submitters should perform experimentation, debugging, and testing in their own repositories. The Klipper Discourse server is a good place to raise awareness of new work and to find users interested in providing real-world feedback.

Submissions must pass all regression test cases.

Code submissions should not contain excessive debugging code, debugging options, nor run-time debug logging.

Comments in code submissions should focus on enhancing code maintenance. Submissions should not contain "commented out code" nor excessive comments describing past implementations. There should not be excessive "todo" comments.

Updates to documentation should not declare that they are a "work in progress".
Does the submission provide a "high impact" benefit to real-world users performing real-world tasks?

Reviewers need to identify, at least in their own minds, roughly "who the target audience is", a rough scale of "the size of that audience", the "benefit" they will obtain, how the "benefit is measured", and the "results of those measurement tests". In most cases this will be obvious to both the submitter and the reviewer, and it is not explicitly stated during a review.

Submissions to the master Klipper branch are expected to have a noteworthy target audience. As a general "rule of thumb", submissions should target a user base of at least a 100 real-world users.

If a reviewer asks for details on the "benefit" of a submission, please don't consider it criticism. Being able to understand the real-world benefits of a change is a natural part of a review.

When discussing benefits it is preferable to discuss "facts and measurements". In general, reviewers are not looking for responses of the form "someone may find option X useful", nor are they looking for responses of the form "this submission adds a feature that firmware X implements". Instead, it is generally preferable to discuss details on how the quality improvement was measured and what were the results of those measurements - for example, "tests on Acme X1000 printers show improved corners as seen in picture ...", or for example "print time of real-world object X on a Foomatic X900 printer went from 4 hours to 3.5 hours". It is understood that testing of this type can take significant time and effort. Some of Klipper's most notable features took months of discussion, rework, testing, and documentation prior to being merged into the master branch.

All new modules, config options, commands, command parameters, and documents should have "high impact". We do not want to burden users with options that they can not reasonably configure nor do we want to burden them with options that don't provide a notable benefit.

A reviewer may ask for clarification on how a user is to configure an option - an ideal response will contain details on the process - for example, "users of the MegaX500 are expected to set option X to 99.3 while users of the Elite100Y are expected to calibrate option X using procedure ...".

If the goal of an option is to make the code more modular then prefer using code constants instead of user facing config options.

New modules, new options, and new parameters should not provide similar functionality to existing modules - if the differences are arbitrary than it's preferable to utilize the existing system or refactor the existing code.
Is the copyright of the submission clear, non-gratuitous, and compatible?

New C files and Python files should have an unambiguous copyright statement. See the existing files for the preferred format. Declaring a copyright on an existing file when making minor changes to that file is discouraged.

Code taken from 3rd party sources must be compatible with the Klipper license (GNU GPLv3). Large 3rd party code additions should be added to the lib/ directory (and follow the format described in lib/README).

Submitters must provide a Signed-off-by line using their full real name. It indicates the submitter agrees with the developer certificate of origin.
Does the submission follow guidelines specified in the Klipper documentation?

In particular, code should follow the guidelines in Code_Overview.md and config files should follow the guidelines in Example_Configs.md.
Is the Klipper documentation updated to reflect new changes?

At a minimum, the reference documentation must be updated with corresponding changes to the code:
- All commands and command parameters must be documented in G-Codes.md.
- All user facing modules and their config parameters must be documented in Config_Reference.md.
- All exported "status variables" must be documented in Status_Reference.md.
- All new "webhooks" and their parameters must be documented in API_Server.md.
- Any change that makes a non-backwards compatible change to a command or config file setting must be documented in Config_Changes.md.
New documents should be added to Overview.md and be added to the website index docs/_klipper3d/mkdocs.yml.
Are commits well formed, address a single topic per commit, and independent?

Commit messages should follow the preferred format.

Commits must not have a merge conflict. New additions to the Klipper master branch are always done via a "rebase" or "squash and rebase". It is generally not necessary for submitters to re-merge their submission on every update to the Klipper master repository. However, if there is a merge conflict, then submitters are recommended to use git rebase to address the conflict.

Each commit should address a single high-level change. Large changes should be broken up into multiple independent commits. Each commit should "stand on its own" so that tools like git bisect and git revert work reliably.

Whitespace changes should not be mixed with functional changes. In general, gratuitous whitespace changes are not accepted unless they are from the established "owner" of the code being modified.

Klipper does not implement a strict "coding style guide", but modifications to existing code should follow the high-level code flow, code indentation style, and format of that existing code. Submissions of new modules and systems have more flexibility in coding style, but it is preferable for that new code to follow an internally consistent style and to generally follow industry wide coding norms.

It is not a goal of a review to discuss "better implementations". However, if a reviewer struggles to understand the implementation of a submission, then they may ask for changes to make the implementation more transparent. In particular, if reviewers can not convince themselves that a submission is free of defects then changes may be necessary.

As part of a review, a reviewer may create an alternate Pull Request for a topic. This may be done to avoid excessive "back and forth" on minor procedural items and thus streamline the submission process. It may also be done because the discussion inspires a reviewer to build an alternative implementation. Both situations are a normal result of a review and should not be considered criticism of the original submission.

Helping with reviews

We appreciate help with reviews! It is not necessary to be a listed reviewer to perform a review. Submitters of GitHub Pull Requests are also encouraged to review their own submissions.

To help with a review, follow the steps outlined in what to expect in a review to verify the submission. After completing the review, add a comment to the GitHub Pull Request with your findings. If the submission passes the review then please state that explicitly in the comment - for example something like "I reviewed this change according to the steps in the CONTRIBUTING document and everything looks good to me". If unable to complete some steps in the review then please explicitly state which steps were reviewed and which steps were not reviewed - for example something like "I didn't check the code for defects, but I reviewed everything else in the CONTRIBUTING document and it looks good".

We also appreciate testing of submissions. If the code was tested then please add a comment to the GitHub Pull Request with the results of your test - success or failure. Please explicitly state that the code was tested and the results - for example something like "I tested this code on my Acme900Z printer with a vase print and the results were good".

Reviewers

The Klipper "reviewers" are:

Name	GitHub Id	Areas of interest
Dmitry Butyugin	@dmbutyugin	Input shaping, resonance testing, kinematics
Eric Callahan	@Arksine	Bed leveling, MCU flashing
Kevin O'Connor	@KevinOConnor	Core motion system, Micro-controller code
Paul McGowan	@mental405	Configuration files, documentation

Please do not "ping" any of the reviewers and please do not direct submissions at them. All of the reviewers monitor the forums and PRs, and will take on reviews when they have time to.

The Klipper "maintainers" are:

Name	GitHub name
Kevin O'Connor	@KevinOConnor

Format of commit messages

Each commit should have a commit message formatted similar to the following:

module: Capitalized, short (50 chars or less) summary

More detailed explanatory text, if necessary.  Wrap it to about 75
characters or so.  In some contexts, the first line is treated as the
subject of an email and the rest of the text as the body.  The blank
line separating the summary from the body is critical (unless you omit
the body entirely); tools like rebase can get confused if you run the
two together.

Further paragraphs come after blank lines..

Signed-off-by: My Name <myemail@example.org>

In the above example, module should be the name of a file or directory in the repository (without a file extension). For example, clocksync: Fix typo in pause() call at connect time. The purpose of specifying a module name in the commit message is to help provide context for the commit comments.

It is important to have a "Signed-off-by" line on each commit - it certifies that you agree to the developer certificate of origin. It must contain your real name (sorry, no pseudonyms or anonymous contributions) and contain a current email address.

Contributing to Klipper Translations

Klipper-translations Project is a project dedicated to translating Klipper to different languages. Weblate hosts all the Gettext strings for translating and reviewing. Locales can be displayed on klipper3d.org once they satisfy the following requirements:

75% Total coverage
All titles (H1) are translated
An updated navigation hierarchy PR in klipper-translations.

To reduce the frustration of translating domain-specific terms and gain awareness of the ongoing translations, you can submit a PR modifying the Klipper-translations Project readme.md. Once a translation is ready, the corresponding modification to the Klipper project can be made.

If a translation already exists in the Klipper repository and no longer meets the checklist above, it will be marked out-of-date after a month without updates.

Once the requirements are met, you need to:

update klipper-tranlations repository active_translations
Optional: add a manual-index.md file in klipper-translations repository's docs\locals\<lang> folder to replace the language specific index.md (generated index.md does not render correctly).

Known Issues:

Currently, there isn't a method for correctly translating pictures in the documentation
It is impossible to translate titles in mkdocs.yml.

15 KiB Raw Blame History