Contributor Info¶
The git repository for the project is hosted on GitHub at chapel-lang/chapel. Anyone can read the repository. It is open source!
This document contains a mixture of tips for git beginners and specific Chapel workflow recommendations.
Below are instructions for setting up a GitHub account, developing a feature, and submitting pull requests.
Note
All commits must be signed according to the DCO (see below) to be merged.
Overview:
Choosing a task¶
If you do not already know what task to work on, the Contributing page has some tips for finding a task.
It is important to communicate with other people before working on a task. This will help address design questions before starting implementation and will avoid multiple people working on the same task simultaneously.
If an issue exists for the task you are working on, you should always comment on that issue to let others know you are working on it. If an issue does not exist for the task you have chosen, you should open an issue first. In many cases, you will need to discuss the design of the interface or implementation before starting development. See the Design section for more details on this.
Design¶
When design discussion is needed¶
Design discussion is necessary when proposing language or standard library changes. It is also necessary for any large or architectural changes.
In many cases it is reasonable to start development without a full design review, but such efforts should be open to changing once the design is being discussed.
If your task does not require a design issue, you may skip ahead to the Development section.
Creating a design issue¶
The design issue should aim to clearly and concisely present some design decisions. Here are some strategies to make a good design issue:
Summarize issue at the top, preferably include concise code example(s).
Try not to make initial issue too long in order to lower the barrier for someone getting involved.
Clearly define the problem this proposal aims to solve.
It can be useful to consider alternative solutions to the problem and describe the trade-offs among them.
A design issue should have two developers not involved in the effort to review.
To find reviewers, post an inquiry to the Chapel Developers Discourse forum.
Use a prefix of
[Design]
in the subject header.Include a short summary of the topic, potentially including motivation and/or an example.
This should not be a copy/paste of the issue contents.
Include a link to your issue.
Ask for people to volunteer to be involved in design discussions.
Subject: [Design] Add feature A to module M Hello, I would like to add feature A to module M. This feature is motivated by X, Y, and Z. For example, this would enable: var result = M.A(args); See the issue here: https://github.com/chapel-lang/chapel/issues/<number> I need 2 developers to identify themselves as API reviewers for this design issue. Thanks, Contributor
Leading a design discussion¶
Many design choices require a consensus among community members. It can be helpful to reach a consensus on a design decision quicker by making an effort to lead that discussion.
Here are some ways to progress the discussion:
Ask people involved what they need to know to make a decision
Summarize the different solutions that were brought up in the discussion and list their pros and cons
Compare with other languages, libraries, previous work in the area
It is common for new design questions to emerge during design discussion (or less commonly, in code review). It can be helpful to spin off new issues for design questions that generate a lot of discussion or design questions that are not completely on-topic. This will help keep the discussion focused and the goals of the current design issue clear.
Development¶
Get set up¶
This should only need to happen once per developer.
Note: these are expected to evolve over time as the governance of Chapel is migrated to an external/community body (the major elements are likely to be similar, though the specific people involved are likely to change and grow).
Set up a GitHub account. The “Free” plan is sufficient for contributing to Chapel.
Use the GitHub web interface to create a fork of the Chapel repo by visiting https://github.com/chapel-lang/chapel and clicking the ‘Fork’ button (see also Fork the repo).
Make sure you have configured your environment to work with git. See Git tips for Chapel developers for details, particularly if you are new to git.
Make sure you understand how to sign your commits with respect to the DCO.
Chapel enforces the Developer Certificate of Origin (DCO) on all pull requests. This requires all commits you make to be signed to indicate that they adhere to the DCO policy. If you’re not already familiar with DCOs, read Getting started with Chapel and the Developer Certificate of Origin (DCO) to learn more about them and how to sign your commits.
If you’re working on a long-term effort, announce it in the Chapel Developers Discourse category to make sure toes are not being stepped on, work is not being pursued redundantly, etc. Similarly, fundamental changes to the language or architecture should be circulated in the Chapel Developers and/or Chapel Users categories to make sure effort is not wasted pursuing an unpopular idea.
You do not need commit/push access to the main repo in order to contribute code. See Who has or needs commit access to the main repository?.
Third-party code requires additional approvals, see the policy details on Third-party code.
Create new branch¶
This should happen once for every new effort.
Develop your feature, bug fix, etc. on your fork. To create a new
branch, use git checkout -b <branch_name>
. Using a concisely
named branch is encouraged.
Develop and test contributions locally¶
Your contribution will take the form of a series of commits. While including sensible commit messages is a good idea, it is more important to have a good merge message once the pull request is going in. Likewise, it is OK to have many small commits that reflect the history of development rather than commits for the feature. Review git Development commands for how to perform some common operations during development.
As you work, you will want to periodically bring in changes from the main Chapel project to your feature branch (described in Development commands), to avoid code drift.
Add new tests¶
You will probably need to create new tests for your feature. See Chapel Testing System for more information on this process.
Any addition/change to the Chapel test system should pass testing when that
test/directory is run with start_test
(and performance tests should also
pass testing for start_test -performance
).
Contributing changes¶
Push your work to your feature branch¶
Push your changes to your feature branch on GitHub to enable others to see your work (see How to push for command details). Note that if you have already created a pull request from a feature branch, pushing your work to that feature branch will update the pull request.
Ask for feedback on your branch early (optional)¶
Not ready to merge your changes, but still want to see if your work is going in the right direction? Feel free to ask for early feedback! Exposing the code is generally done by:
Pointing someone to your feature branch on GitHub, or
Creating a Work-in-progress pull request for your feature branch. See the Submit pull request section below for how to do this.
Discussion can take place in:
a separate GitHub issue
the Chapel Developers Discourse forum
a private communication
some other strategy agreed upon by all involved parties
Submit pull request¶
See How to open a PR: for the sequence of steps necessary.
Contributors should be reasonably confident in the testing done on their code before asking for a final review. Should additional testing resources be needed, you can request help from a member of the core Chapel team when creating your pull request.
In working with your reviewers, you will no doubt change your pull request. Just do your local development and then update your feature branch as in Push your work to your feature branch
Please follow the Pull request guidance and keep PRs reasonably sized.
Find a reviewer¶
Once your PR is ready, you’ll need to request a review. If you know who you’d like to review it, @ mention them in a comment on the PR and ask them to have a look. If you don’t know their GitHub id, you can find them in the chat room or send them an email. If you don’t know who should review the change, post to the Chapel Developers Discourse topic requesting a review and linking to the PR. By convention, such topics often use a [PR] prefix in their titles.
Note: Ideally, someone should volunteer to review your pull request within a day or two. If this doesn’t happen, feel free to make some noise. Ideally the review should take place within a few days, though timing may vary depending on other deadlines.
See Reviewer responsibilities for details on what performing a review on another contributor’s code entails.
Work with your reviewers¶
Iterate with the reviewer until you’re both satisfied. One should generally try to do whatever their reviewer asks. Sometimes, a reviewer will ask for something really hard. Try to make sure they understand the magnitude of the request, and try to discuss if it’s really necessary to do before merging. If you can’t come to an agreement, one of you should bring other developers (individually or via the Chapel Developers forum) into the conversation to get a broader opinion. One of the jobs of the reviewer is to serve as a proxy for other developers, or to bring those developers into the conversation if they feel unqualified to do so.
Before merging¶
Before the change can be merged, go through this checklist to ensure:
all design changes have been discussed
all commits contain the required “Signed-off-by:” line to indicate compliance with the Developer Certificate of Origin (DCO – see Getting started with Chapel and the Developer Certificate of Origin (DCO))
the PR has been reviewed
the Git history is clear of anything that should not be in the repo
relevant configurations pass testing
If you did not have the resources to perform at least a full correctness test run, this is the point at which a reviewer would do so for you. Contributors are expected to have verified any new tests work before asking a reviewer to do this.
Details on how to run tests may be found at Test System, and details on the appropriate amount of testing before merging the final PR may be found at Testing your patch.
Once the pull request is approved, it can be merged. This can be done by either the reviewer or developer (given sufficient permissions), as decided between the two of them. See How to merge a PR: for steps to perform this.
If you are reviewing code from an external contributor without push privileges, go through the checklist once more before merging the change.
After the final version of the change has been agreed upon, the person making the merge should follow the steps for How to merge a PR:.
After merging¶
After merging, a good contributor should watch automatic testing results for failures that they may need to address:
In short order, a smoke-test will be run against the commit to make sure that nothing basic has been broken by it. Your reviewer should alert you to any problems.
After the commit has gone in, check with your reviewer to ensure that there are no new failures caused by your commit in our nightly testing.
Other useful information¶
Policy details¶
Who has or needs commit access to the main repository?¶
Core team members have commit access to the main repository. Reviewers on the core team can pull, review, and merge your pull requests. Even the developers that have write access to the Chapel repository need to have all non-trivial changes reviewed. Developers who have been given write access can merge trivial changes (e.g. small bug fixes, documentation changes) without review.
If you will need commit/push access to the main repository, chapel-lang/chapel, send a request including your GitHub username to chapel_admin _at_ cray.com.
Third-party code¶
If your work will require committing any third-party code that you are not developing yourself (or code that you’ve developed as a standalone package), alert the Chapel Developers Discourse forum of this as, presently, such code packages must be approved before being committed.
Here are some guiding questions to determine whether a third-party package you rely on should be committed to the chapel repository:
How large is the third-party code you wish to include?
If the code is very large, perhaps it would be better to add directions on how to install this dependency.
Under what license does this code operate?
We try not to add dependencies on code that is under GPL or LGPL, as those licenses have copyleft properties and force derivative works to be distributed under the same license.
Is there an alternate package with a more permissive license that can accomplish the same purpose?
If so, we recommend relying on that package instead.
If not, it would be better to add directions on how to install this dependency.
How easy is this code to obtain?
Will it be installed by default on an ordinary machine?
If so, we do not need to redistribute it ourselves.
How much of the Chapel implementation will rely on this code?
The compiler for ordinary Chapel? A commonly used runtime configuration?
In these cases, we will probably want to include the code in our distribution.
A standard or package module that is not included by default?
Depending on the circumstances, it might be better to just include directions on how to install this code.
Do we require Chapel-specific modifications to the code in order to use it?
If so, we will probably want to distribute this package, or at least include the modifications and an easy way to install them.
Please include the answers to these questions when you post to the Chapel Developers forum, if you believe the code should be included or you remain uncertain.
Testing your patch¶
Changes to the Chapel implementation should not cause regressions. Developers are responsible for doing a degree of testing that’s appropriate for their change (described in the following bullets) and then can rely on nightly regression testing to worry about the full cross-product of configurations.
At a minimum, patches should pass correctness testing for the full test/ directory hierarchy for:
CHPL_*_PLATFORM=linux64
CHPL_*_COMPILER=gnu
CHPL_COMM=none
CHPL_TASKS=<default>
Most developers will start by focusing on a subdirectory of tests that exercise the features they changed, or test/release/ as a suite of tests that exercises a rich and important slice of the language.
Changes that are likely to affect multi-locale executions should also be tested against tests that exercise multi-locale capabilities with
CHPL_COMM=gasnet
. A common subset is:test/release/
,test/multilocale/
, andtest/distributions/
.Changes that are likely to cause portability issues should be tested against different platforms and compilers to avoid fallout in the nightly testing to the extent possible.
Note that the quickest way to do testing is to use the parallel testing system across a large number of workstations. If you have limited testing resources available to you, you can request that a member of the core Chapel team help.
Work-in-progress pull request¶
A work-in-progress (WIP) PR is a special kind of pull request that is not yet intended to be merged. Such PRs are created to take advantage of what the GitHub PR interface provides, such as public comment history and quick links between the WIP PR and other related issues and pull requests. They allow the developer to get early feedback on a change.
The status of a WIP PR can be stated by using the GitHub “draft PR” feature.
The PR description should include what steps need to be taken before the PR is
ready for final review. If a WIP PR was not opened as draft PR, it can be
marked with the stat: work in progress
label by a core contributor.
It is perfectly acceptable to abandon such PRs (especially in favor of a cleaned up version of the code) when the git history becomes too large, so long as a link to the original PR is provided when the change is eventually merged, to preserve the discussion.
Pull request guidance¶
It is considered good practice to keep PRs (pull requests) to a reasonable size. This ensures that the PR will be reviewed in a timely manner and will receive a higher level of attention per line of code during review.
When submitting a PR, the contributor should ask themselves if their contribution can be separated into smaller logical chunks or independent parts. Reviewers will also be pondering the same question and may request a break up of the contribution into smaller PRs.
Breaking up a PR can sometimes require a great deal of effort and creativity, and may not be feasible at all, due to the intertwined nature of the code.
Ideally, the size of the PR should be proportional to the expected value to the developer and user community.
Pull request description¶
Generally, try to come up with a single-line headline of 75 characters at most to describe the change. Think of this as a statement of what the PR accomplishes, ideally with a user perspective (as opposed to the “what I did to the code” perspective).
Depending on the magnitude of the change, write a short paragraph to longer set of paragraphs describing the change (again, focusing on the user experience, at least in the initial paragraphs… why would a Chapel user care that you merged this? Or if it’s not user-facing, why would a developer care).
If it’s truly complex such that you think the code changes themselves warrant describing in the PR (because they’re complex or clever, say), additional paragraphs talking about the code changes / approach taken can be nice (but aren’t strictly required).
Depending on the complexity of the PR, it can be reassuring to reviewers to denote how you have tested the PR either in the PR description or an initial comment.
It’s not uncommon for the contents of the PR description to evolve over the lifetime of the PR and its review. Initially it should help the person doing the code review understand what’s being done (and potentially why). Then, when the code review is done, make sure the text is still accurate.
Final merge message¶
When merging, copy and paste the PR description into the GitHub merge dialog box. Ensure that the title of the PR is at the top.
It is good practice to @-mention the reviewer(s) at the end of the PR. This is typically formatted in brackets:
[Reviewed by @reviewer1 and @reviewer2]
Git history is clear¶
In general, having logical commits with meaningful commit messages is helpful for maintaining a clean git history. This is particularly important for critical or complex code changes. Depending on how critical or complex your changes are, it may be a good idea to do an interactive rebase to squash any non-meaningful commits:
git fetch upstream
git rebase -i upstream/main
Note that this can be particularly cumbersome when there has been significant conflicting changes made on upstream main, so it is not a hard requirement.
It’s not generally possible to completely remove a commit from git by the time it makes it in to the main branch. So be very careful not to commit anything that you might regret later (e.g., sensitive code, code owned by anyone other than yourself). Ideally, the review will catch such issues, but the ultimate responsibility is on the developer.
Reviewer responsibilities¶
Test the PR you are reviewing. Follow the instructions next to the “Merge pull request” button in the “command-line instructions” link to check out a local copy of the PR for testing. They will look something like the following:
git checkout -b contribusername-prbranchname main
git pull https://github.com/contribusername/chapel.git prbranchname
# don't forget to recompile before you test
If you’re reviewing a commit from a developer outside the Chapel core team, be sure their commits are signed via the DCO bot (one of several github action checks that will run on each PR). If they’re not, help the developer understand the requirement.
Care may need to be taken when committing third-party code that originates from a different git[hub] repository. There is a contributor’s agreement that might be needed. We may also need to do a licensing review. Have a discussion with core developers before doing this.
What Copyright Should I Use?¶
By opening a PR with signed commits, you are agreeing that code you contribute will be governed by the license and copyright of the project as a whole. A standard block of license text is required at the top of every compiler, runtime, and module code file. Browse other files of the same type to see the required license block.
Additional copyrights may also be applied, as appropriate.
Chapel release process¶
When a Chapel release nears, merging of pull requests is temporarily frozen for a few weeks to ensure stability of the release. These freezes come in two phases: feature freeze and code/doc freeze.
After feature freeze, only bug fixes and documentation updates will be accepted for the upcoming release. After code/doc freeze, no contributions will be accepted for the upcoming release, barring critical bug fixes. At some point between code/doc freeze and the release, a release branch will be “cut”, and the main branch will begin accepting all contributions again.
The dates for these freezes will be posted on the Chapel events page sometime close to each release.