ChiliProject is not maintained anymore. Please be advised that there will be no more updates.
We do not recommend that you setup new ChiliProject instances and we urge all existing users to migrate their data to a maintained system, e.g. Redmine. We will provide a migration script later. In the meantime, you can use the instructions by Christian Daehn.
When reviewing code to be included there are a few goals to keep in mind:
Code quality questions¶
- does the code work?
- is the code readable?
- is the code better than the code around it?
- are boundary conditions handled?
- does it follow MVC?
- user input sanitized?
- any potential security problems? Rails Security Guide
- follow the Code Standards?
- all of the tests pass?
- new tests added?
- existing tests changed?
- using fixtures (bad) or factories (good) used?
- do tests have a clear name?
def test_this_thingis bad,
test "User#activate should activate the user"is good.
- edge cases tested? (See below)
- is there user documentation that needs to be created?
- are there rdocs/yardocs for the code?
- all user interface strings using the I18N system?
- all labels attached to the form fields?
- runs on Ruby 1.8.7 and 1.9.2?
- runs on Rails 2.3.5?
- change public internal APIs that haven't be deprecated yet? (e.g. changing
- add deprecation warnings when changing change public internal APIs?
- add or change any external dependencies? (judgement call, we don't want to write everything ourselves and should rely on stable and high quality third party code if it helps)
Testing edge cases¶
It's difficult to test all of the edge cases but check that the major ones are handled:
- most common action
- low boundary conditions (e.g. test 0 if a field should have a positive number)
- high boundary conditions (e.g. test 99 if a field should have a two digit positive number)
- just outside the boundary conditions (e.g. test -1 if a field should be positive)
- nil/null/empty array/empty hash conditions
Also if the code is about user accounts, permissions, or authentication then all of the different user states need to be tested:
- Anonymous user
- Normal user, without project membership
- Normal user, with project membership
- Admin user, without project membership
- Admin user, with project membership
- Archived user
Code review process¶
- TODO: github pull request review
- TODO: issue attachment review
- TODO: github forks patch review
- TODO: upstream code reviews
- TODO: security code reviews and audits