MoSeq Developer Guidelines

Git Branching Model

We keep two main permanent branches, release and dev, to keep track of the code release and development. The release branch has the most updated stable version of the code and users should use the numbered releases, which are only made from the release branch. The dev branch has all the most updated delivered development changes, and it reflects the code that is staged for inclusion in the next stable release.

All new additions (bug fixes, features, chores, docs, etc.) should branch off dev and then merge back to dev. External developers should fork the repositories before making any changes to the code. Developers should always check their dev branch is up to date with the upstream branch with git merge upstream/dev. It is strongly advised that developers should ensure their branches are up to date before making any changes to minimize chances of merge conflicts when opening a PR. If there are any changes, the developers are responsible for resolving the changes. If the developer has forked the repositories, please sync the fork periodically to include the latest changes.

Git Branching Model Diagram (Graph modified from here)

Branch Naming Conventions

Developers should always checkout a new branch based on the dev branch to work on the code. To keep track of the purpose of the branches and accelerate the code review process, it is advised to name the branches with a short description of the task. The words should be joined with hyphens. For example, fix-installation-script, add-scalar-descriptor. You can read more about branch naming conventions here.

Conventional Commits

Developers should follow Conventional Commits to make commits more informative. We normally use tags such as fix, feat, docs, and chore to signify the purpose of the commits, and use ! in the comment to signify breaking changes. It is recommended to commit frequently so the progress of coding is tracked. For example fix: crowd movie comparison key error when reading a bigger data frame, feat: added duration field to interactive tools

Pull Request (PR) Conventions

The name of the Pull Request should follow Conventional Commits and summarize the purpose of the PR. Developers should fill in the following markdown template to construct a detailed description of the PR. Developers should NEVER use --force to force pushes. If the PR is associated with an issue, the developer should signify the issue number in the PR description.** If the developer has forked our repositories, we recommend fetching upstream using git fetch upstream followed by git merge upstream/dev before making changes so the fork is up to date. **It’s possible to sync the fork after making changes, but doing that may result in duplicated work and unexpected changes. When syncing the fork after making changes, first run git fetch upstream then merge dev branch into the branch that the PR is for. We don’t recommend using git rebase as it may involve a force push.

PRs from external developers will only be reviewed when all the CI tests pass and the necessary tests are added. The PR may get merged into the dev branch after code review. When merging, the merge should also follow Conventional Commits. A PR template will be generated automatically when you submit a PR and please use the template.

CI Checks and Tests

We use Travis CI and the CI tests will be automatically triggered when external developers submit PRs and internal developers commit changes. We use pytests to write unit/functional tests. Developers should test the code locally with pytest. New features must be accompanied by appropriate tests before they can be merged. The tests are ./test folder and triggered by pytest.ini. New tests should be added to ./test folder.

Follow the steps to run the test suites locally:

1. Run pip install pytest==5.4.1 codecov pytest-cov to download necessary packages for testing.
2. Run ./scripts/download_test_data.sh to download test data.
3. Run pytest --cov-report xml --cov-report term to run the tests.

You only need to run step 1 once for each environment. Test data are downloaded to data folder and because data folder is in .gitignore, you only need to run step 2 once for each repository.

Code Review

We follow the code review process in Google's Engineering Practices. When submitting a PR, at least one of the internal MoSeq team developers should review the code and give feedback. If the PR is not ready, the reviewer MUST Request Changes so that the PR can’t be merged into the branch without the changes being addressed.

Common Anti-Patterns to Avoid MoSeq is based on Python and here are some of the common anti-patterns that developers should avoid:

1. Opening file without a context manager
2. Try… Except… block with lazy exception handling (Pass). All exceptions should be handled with care and users should be notified when the program fails.
3. Returning more than one object type in a function.
4. Not using the get method for dictionaries.
5. Not cleaning debugger code (print statements for debugging purposes)

You can read more about anti-patterns here.

GitHub Issues

We encourage users to submit bug reports and feature requests here when there is a bug or a desired feature that is not implemented. We encourage you to check out the troubleshooting and tips section and search your issues in existing issues first.

Feedback

Please join our Slack channel for more discussion!