Contributing to Projects
- Basic Requirements
- Describe Your Changes
- Separate Your Changes
- Style-Check Your Changes
- Include a Test
- Respond to Review Comments
- Editor Settings
- Testing
Vaadin’s open source projects are hosted on GitHub and released under the Apache 2.0 license. If you would like to contribute something, or want to modify the code for your own use, this document should help you get started.
All bugs and enhancements are tracked in corresponding GitHub issues. If you are unsure where to start, you can pick one of the issues marked with "Help Wanted" or "Good First Issue" labels. The Vaadin Forum is a great place to connect with others in the community, including some of the Vaadin team members. You can also ask questions there. If you are reporting a bug, you can help to speed up problem diagnosis by providing as much information as possible. Ideally, that would include a small sample project that reproduces the problem.
Helping to review pull requests is another great way to contribute. Your feedback can help to shape the implementation of new features. When reviewing pull requests, however, refrain from approving or rejecting a PR.
Basic Requirements
All contributions should target the master
branch.
Project teams picks the changes to correct version branches from the master
.
Important
|
Requirements for new features and enhancements
To ensure maintainability, product quality and to avoid API inconsistencies it’s important that all enhancements and new features are first discussed in a GitHub issues.
Project teams can also help you with acceptance criteria (a set of user stories), that states what’s the expected use-case.
|
A quality patch follows good coding practices - it’s easy to read and understand. For more complicated fixes or features, break down the change into several smaller easy to understand patches.
Describe Your Changes
- Subject line
-
Start with a good subject message in imperative form with 50 chars or less. A well formed Git PR subject line should always be able to complete the following sentence: "If applied, this PR will <your subject line here>"
Depending of type of changes you are doing, the subject line should start with
feat
,fix
,chore
orrefactor
. In case there are breaking changes, put!
after the prefix, likerefactor!:
. If you aren’t sure what to write there, let the reviewer do it when merging the PR.feat: Create a Valo icon font for icons in Valo
- Describe the problem
-
Whether your patch is a one-line bug fix or 5000 lines of a new feature, there must be an underlying problem that motivated you to do this work. Convince the reviewer that there is a problem worth fixing and that it makes sense for them to read past the first paragraph. This is often already described in bug/enhancement issue, but also summarize it in your commit message.
Valo uses only a handful of icons from Font Awesome.
- Describe the user impact and solution
-
Straight up crashes and lockups are pretty convincing, but not all bugs are that blatant. Even if the problem was spotted during code review, describe the impact you think it can have on users.
Once the problem is established, describe what you are actually doing about it in technical detail. It’s important to describe the change in plain English for the reviewer to verify that the code is behaving as you intend it to. Describe your changes in imperative mood, for example "make X do Y". If the patch fixes a logged bug entry, refer to that bug entry by number or URL. However, try to make your explanation understandable without external resources.
If your description starts to get long, that’s a sign that you probably need to split up your patch. See “Separate Your Changes”.
This change introduces a separate icon font for valo (9KB instead of 80KB) and decouples Valo from Font Awesome to enable updating Font Awesome without taking Valo into account. This change also makes it easy to not load Font Awesome when using Valo by setting $v-font-awesome:false For backwards compatibility, Font Awesome is loaded by default
- Reference the issue
-
Reference an issue number using the magic words to close the issue:
If the issue isn’t closed by this PR, you can still refer to it with the syntax:
Part of #1234
. In case the issue is in another repository, you can link to it with the syntax:Part of vaadin/spring#1234
where the first part is for the organization, the second for the repository followed by the issue (or PR) number there.Fixes #18472
- Complete PR message example
feat: Create a Valo icon font for icons in Valo
Valo uses only a handful of icons from Font Awesome. This change introduces a separate icon font for valo (9KB instead of 80KB) and decouples Valo from Font Awesome to enable updating Font Awesome without taking Valo into account.
This change also makes it easy to not load Font Awesome when using Valo by setting $v-font-awesome:false
For backwards compatibility, Font Awesome is loaded by default.
Fixes #18472
Separate Your Changes
Separate all enhancements, fixes, and new features into different pull requests.
For example, if your changes include both bug fixes and performance enhancements, separate those changes into two or more patches. If your changes include an API update, and a new component which uses that new API, separate those into two patches.
If you make a single change to several files, group those changes into a single patch when possible. Thus a single logical change is contained within a single patch.
If one patch depends on another patch in order for a change to be complete, that’s OK. Add note "this patch depends on patch X" to your patch description.
When dividing your change into a series of patches, take special care to ensure that the project builds and runs after each patch in the series. Compilation failures are especially annoying to deal with.
Style-Check Your Changes
Check your patch for basic style violations. There should be none if you have setup your project following the instructions.
If you are touching old files and want to update them to current style conventions, do so in a separate commit/PR. It’s best to have this commit as the first in the series.
Include a Test
Where applicable, patches should be accompanied with automated tests. It allows to detect regressions during the build thus simplifying future maintenance. Unit tests are the easiest to implement, but certain aspects (changes to the UI or session management code) might require an integration test.
After submitting a pull request, CI system triggers the verification build automatically, including integration tests, and reports results to the PR.
Test cases should succeed with the patch and fail without the patch. This is a clear indication that the suggested fix/enhancement does what expected.
If the patch is aimed at the performance improvement, supplement it with a performance test code and a benchmark results showing performance impact.
Respond to Review Comments
Code review is an essential part of PR acceptance process and is often a logical continuation of a discussion started in a GitHub issue. Don’t be offended if reviewer asks you to change the implementation or use a different approach. Such changes are often required to align API with a new features being actively developed and to ensure backward-compatibility.
It’s best to keep the conversation going in review comments and resolve all reviewer comments. If the PR isn’t approved by the reviewer and there is no response from the author in a reasonable time, PR is likely to be rejected as abandoned.
Another aspect to consider is that, as time passes, more and more new features and fixes are merged into the master
branch.
As a result, the more PR is waiting to be merged, the higher is the probability of merge conflicts.
Such conflicts must be resolved before the merge.