-
Notifications
You must be signed in to change notification settings - Fork 16
Overview
We don't like process, but it's necessary for coordinating a development team. We have multiple goals for our developer process:
- Ensure the proposed changes are maintainable
- designed properly
- uses consistent Coding style
- documented
Issues should be focused and have limited scope. Each issue should conceptually be one thing. It is common for a project to spawn multiple issues. It is also common to have to start work on one issue and then generate multiple issues. Use your best judgement, but in most cases, smaller issues are better than bigger issues.
Create a fork of the latest master branch of the repo. It is absolutely essential that you create a fork of the repository and not clone it since you'll not be able to push the changes you have made in case you have cloned the repository.
If you have an existing fork, we recommend updating it to the current master branch of chunkie or creating a new fork. In order to update your existing fork:
- Add chunkie and call it upstream
git remote add upstream https://github.com/fastalgorithms/chunkie.git
- Fetch all branches of remote chunkie
git fetch upstream
- Rewrite your master using chunkie's master using git's rebase
git rebase upstream/master
- Push updates to your master or other branches
git push origin master --force
Note that this may overwrite local changes you have on your fork. In case you have local changes, creating a new fork may be a safer alternative.
In your local fork of the chunkie repo, create a branch from master. Replace the # below with the issue number from above.
If the issue is a bug, then name the branch: bugfix/issue-#-short-description.
If the issue is a feature, then name the branch: feature/issue-#-short-description.
This should look like:
git checkout -b feature/issue-#-short-description
Fix the issue on the branch that was created. This branch will be used to create a pull request.
Please limit the fixes on the branch to the issue. If you find yourself fixing other things on the branch: stop, create a new issue, and follow the process to fix that separate issue. Multiple conceptual fixes together on one branch will slow down the time it takes for it to get into the code base and may not get in as-is.
If you're fixing a bug, first create a test that demonstrates the bug. Commit that test (git commit so it gets added to the history). Then fix the issue. The test failure should go away. This test is a necessary condition for getting the patch in. Tests should be placed in the devtools/test folder. A good naming convention for bug tests is functionname_shortnameforbugTest.m. All tests files should end in "Test.m".
If you're creating a new feature, then every new piece of code should be tested (with exceptions). Any sections that touch existing code should be tested. A good naming convention for new functions is functionnameTest.m.
Tip: The commit message for the final commit for the issue should have fixes #N where N is the issue number; that will make sure the issue is closed when the pull request is merged (you can also use closes or resolves in place of fixes).
Before a pull request can make it into chunkie, it needs to be up to standards. Not all of these apply for every pull request, but for most, it does:
- New / changed code must be tested. There should be new tests.
- All existing tests must pass: in the devtools/test folder run
runtests
- All new / changed code must be documented.
- Code should adhere to the requirements listed in Coding style
Once all of these are taken care of, create a GitHub pull request using the GitHub web interface. You will have to push your branch to origin (GitHub fork) first in order for the web interface to know about your branch. Fill out the Pull Request Template. Then hit submit.
The first stage of code review is fully automated. The pull request will trigger GitHub to run the unit tests. You can see the progress of this on the GitHub webpage for the pull request.
If the unit tests do not pass, the pull request will not be merged and the problem must be fixed (if there is an error in the unit test itself this should be fixed and documented).
If the unit tests pass, someone on the chunkie development team will look at the code for all of the parameters above.
That should be pretty straightforward.
Before merging, make sure:
- tests pass
- someone on the core team has reviewed the pull request thoroughly
- someone on the core team has approved of the pull request