|
| 1 | +# Development Guide |
| 2 | + |
| 3 | +## Table of Contents |
| 4 | +- [Code Review Best Practices](#code-review-best-practices) |
| 5 | +- [Issue Management](#issue-management) |
| 6 | +- [Collaborative Coding Guidelines](#collaborative-coding-guidelines) |
| 7 | +- [Code Formatting](#code-formatting) |
| 8 | + |
| 9 | +## Code Review Best Practices |
| 10 | + |
| 11 | +### For Reviewers |
| 12 | + |
| 13 | +1. **Be Constructive and Respectful** |
| 14 | + - Focus on the code, not the person 🔐 |
| 15 | + - Provide clear, actionable feedback 🚧 |
| 16 | + - Explain the "why" behind your suggestions |
| 17 | + - Use a positive and encouraging tone 💜 |
| 18 | + |
| 19 | +2. **Review Checklist** |
| 20 | + - Code follows [project style guidelines](https://github.com/AdaptiveMotorControlLab/WorkspaceTemplate/blob/mwm/dev-guide/docs/StyleGuide.md) |
| 21 | + - No obvious bugs or security issues |
| 22 | + - **Tests** are included and pass |
| 23 | + - Documentation is updated |
| 24 | + - Performance considerations are addressed |
| 25 | + - Error handling is appropriate |
| 26 | + |
| 27 | +3. **Timely Reviews** |
| 28 | + - Respond to review requests within 24 hours |
| 29 | + - If you can't review immediately, communicate your timeline |
| 30 | + - Don't let PRs sit idle for extended periods |
| 31 | + |
| 32 | +4. **Focus Areas** |
| 33 | + - Functionality: Does it work as intended? |
| 34 | + - Maintainability: Is the code clean and well-structured? |
| 35 | + - Testability: Is it easy to test? |
| 36 | + - Security: Are there any security concerns? |
| 37 | + - Performance: Are there any performance implications? |
| 38 | + |
| 39 | +### For Authors |
| 40 | + |
| 41 | +1. **Before Submitting** |
| 42 | + - Make a branch and adhere to the naming convention: `yourname/issue` |
| 43 | + - Self-review your changes |
| 44 | + - Run all tests locally |
| 45 | + - Run the `pre-commit` checks & formatting! |
| 46 | + - Update documentation! |
| 47 | + - Keep changes focused and manageable -- "MVP-R" what is the minimally viable PR -- would you want to review it? |
| 48 | + - Follow the project's commit message conventions |
| 49 | + |
| 50 | +2. **PR Description** |
| 51 | + - Clear title describing the change |
| 52 | + - Detailed description of changes |
| 53 | + - Link to related issues |
| 54 | + - Screenshots for UI changes |
| 55 | + - Testing instructions |
| 56 | + |
| 57 | +## Issue Management |
| 58 | + |
| 59 | +### Creating Issues |
| 60 | + |
| 61 | +1. **Issue Template** |
| 62 | + - Use the provided issue templates |
| 63 | + - Fill out all required sections |
| 64 | + - Be specific and detailed |
| 65 | + |
| 66 | +2. **Issue Types** |
| 67 | + - Bug: Describe the current behavior and expected behavior |
| 68 | + - Feature: Explain the use case and benefits |
| 69 | + - Enhancement: Detail the improvement |
| 70 | + - Documentation: Specify what needs to be updated |
| 71 | + |
| 72 | +3. **Issue Content** |
| 73 | + - Clear, descriptive title |
| 74 | + - Detailed description |
| 75 | + - Steps to reproduce (for bugs) |
| 76 | + - Expected vs actual behavior |
| 77 | + - Environment details |
| 78 | + - Screenshots/videos when relevant |
| 79 | + |
| 80 | +### Managing Issues |
| 81 | + |
| 82 | +1. **Labels** |
| 83 | + - Use appropriate labels (bug, enhancement, etc.) |
| 84 | + - Priority labels when necessary |
| 85 | + - Component/area labels |
| 86 | + |
| 87 | +2. **Assignments** |
| 88 | + - Assign issues to specific team members |
| 89 | + - Set realistic deadlines |
| 90 | + - Update status regularly |
| 91 | + |
| 92 | +3. **Communication** |
| 93 | + - Keep discussions in the issue thread |
| 94 | + - Tag relevant team members |
| 95 | + - Update issue status promptly |
| 96 | + |
| 97 | +## Collaborative Coding Guidelines |
| 98 | + |
| 99 | +Each project is different, so please check project-specific guidelines. |
| 100 | +However, below is a guide for collaborative projects in general. |
| 101 | +I recommend the following system for within-lab projects that have different levels of maintainers & builders. |
| 102 | + |
| 103 | +### Who is developing? |
| 104 | + |
| 105 | +1. **Organize weekly dev meetings** |
| 106 | + - Review the current issues, PRs, and major milestones. |
| 107 | + - Self-assessment: are you blocking anyone? If so, work to fix that. |
| 108 | + - No one person is the gate-keeper for the project: work together |
| 109 | +2. **Get a review assignment system in place** |
| 110 | + - 🟥 Make a flag for **major dev/changes**: all users of the code should agree and sign off (git reviews), and this includes the PI. |
| 111 | + - 🟧 Make a flag for **user-needs**: this is needed to stop a block -- it might not be perfect, so make an issue to revisit later. 1 sign off from another user, and go! 🚀 |
| 112 | + - 🟩 Make a flag for **minor change**: not breaking, can be changed later, 1 sign off okay |
| 113 | + |
| 114 | +### Git Workflow |
| 115 | + |
| 116 | +1. **Branching Strategy** |
| 117 | + - Use feature branches for new work |
| 118 | + - Branch naming convention: `name/issue-number-description` |
| 119 | + - Example: `john/123-add-login-feature` |
| 120 | + - Example: `sarah/456-fix-navigation-bug` |
| 121 | + - Keep branches up to date with main |
| 122 | + - Delete branches after merging |
| 123 | + |
| 124 | +2. **Commits** |
| 125 | + - Write clear, descriptive commit messages |
| 126 | + - Keep commits focused and atomic |
| 127 | + - Reference issue numbers in commits |
| 128 | + - Follow conventional commits format |
| 129 | + |
| 130 | +3. **Pull Requests** |
| 131 | + - Keep PRs small and focused |
| 132 | + - Update PR description as changes are made |
| 133 | + - Request reviews from appropriate team members |
| 134 | + - Address review comments promptly |
| 135 | + |
| 136 | +### Communication |
| 137 | + |
| 138 | +1. **Team Communication** |
| 139 | + - Use appropriate channels for different purposes - use basecamp campfire and github issues/PRs |
| 140 | + - Be clear and concise |
| 141 | + - Document important decisions!! |
| 142 | + - Share knowledge and learnings |
| 143 | + |
| 144 | +2. **Code Documentation** |
| 145 | + - Document complex logic in the code with comments! |
| 146 | + - Keep README up to date |
| 147 | + - Add inline comments when necessary |
| 148 | + - Document API changes |
| 149 | + |
| 150 | +3. **Knowledge Sharing** |
| 151 | + - Share learnings with the team |
| 152 | + - Document common patterns |
| 153 | + - Create and maintain team documentation |
| 154 | + - Regular team sync-ups, organize them! |
| 155 | + |
| 156 | +### Development Environment |
| 157 | + |
| 158 | +1. **Setup** |
| 159 | + - Document environment setup |
| 160 | + - Use consistent development tools |
| 161 | + - Share configuration/docker files |
| 162 | + - Maintain development dependencies |
| 163 | + |
| 164 | +2. **Local Development** |
| 165 | + - Follow our development guidelines |
| 166 | + - Use consistent formatting tools |
| 167 | + - Run tests before committing (see `pre-commits`) |
| 168 | + - Keep dependencies updated |
| 169 | + |
| 170 | +3. **Code Quality** |
| 171 | + - Use our suggeested linters and formatters |
| 172 | + - Follow our style guides |
| 173 | + |
| 174 | + |
| 175 | +## Code Formatting |
| 176 | + |
| 177 | +### General Formatting Rules |
| 178 | + |
| 179 | +We use the Google Style Guide: https://google.github.io/styleguide/ |
| 180 | + |
| 181 | +1. **Indentation** |
| 182 | + - Use 2 spaces for indentation |
| 183 | + - No tabs |
| 184 | + - Maximum indentation level: 4 levels |
| 185 | + - Align with opening delimiter |
| 186 | + |
| 187 | +2. **Line Length** |
| 188 | + - Maximum line length: 100 characters |
| 189 | + - Break long lines at logical points |
| 190 | + - Indent continuation lines by 4 spaces |
| 191 | + - Break after operators, not before |
| 192 | + |
| 193 | +3. **Whitespace** |
| 194 | + - No trailing whitespace |
| 195 | + - One blank line between functions/classes |
| 196 | + - Two blank lines between major sections |
| 197 | + - No multiple consecutive blank lines |
| 198 | + - Spaces around operators |
| 199 | + - No spaces inside parentheses |
| 200 | + - Spaces after commas and semicolons |
| 201 | + |
| 202 | +4. **Naming Conventions** |
| 203 | + - Variables: `lower_snake_case` |
| 204 | + - Functions: `lower_snake_case` |
| 205 | + - Classes: `PascalCase` |
| 206 | + - onstants: `UPPER_SNAKE_CASE` |
| 207 | + - Files: `lower_snake_case.py` |
| 208 | + - Private members: `_single_leading_underscore` |
| 209 | + - Modules/Packages: `lower_snake_case` |
| 210 | + - Interfaces: `PascalCase` (same as classes, no special prefix) |
| 211 | + |
| 212 | +5. **Comments** |
| 213 | + - Use clear, concise comments |
| 214 | + - Comment complex logic |
| 215 | + - Keep comments up to date |
| 216 | + - Remove commented-out code |
| 217 | + |
| 218 | +6. **Imports/Exports** |
| 219 | + - Group imports in the following order: |
| 220 | + 1. Third-party imports |
| 221 | + 2. Project imports |
| 222 | + - Sort imports by `isort` within the groups |
| 223 | + - Use named exports |
| 224 | + - Avoid default exports |
| 225 | + - One import per line |
| 226 | + |
| 227 | +7. **Code Organization** |
| 228 | + - One class/component per file |
| 229 | + - Group related functionality |
| 230 | + - Keep files focused and manageable |
| 231 | + - Follow the project's file structure |
| 232 | + - RECOMMENDED: Maximum file length: 1000 lines |
| 233 | + - RECOMMENDED: Maximum function length: 50 lines |
| 234 | + |
| 235 | + |
| 236 | +### Automated Formatting |
| 237 | + |
| 238 | +1. **Pre-commit Hooks** |
| 239 | + - Run formatters before commit |
| 240 | + - Run linters before commit |
| 241 | + - Check for common issues |
| 242 | + - Ensure consistent formatting |
| 243 | + - Block commits with formatting errors |
| 244 | + |
| 245 | +2. **CI/CD Integration** |
| 246 | + - Run formatting checks in pipeline |
| 247 | + - Fail builds on formatting errors |
| 248 | + - Generate formatting reports |
| 249 | + - Enforce style guide compliance |
| 250 | + - Use Google's style guide linters |
| 251 | + |
| 252 | +### Google Style Guide Compliance |
| 253 | + |
| 254 | +1. **General Principles** |
| 255 | + - Be consistent with existing code |
| 256 | + - Be consistent with Google's style guide |
| 257 | + - Use Google's official linters |
| 258 | + - Follow language-specific style guides |
| 259 | + - Document any style guide exceptions |
| 260 | + |
| 261 | +2. **Code Review Requirements** |
| 262 | + - Check style guide compliance |
| 263 | + - Verify formatting is correct |
| 264 | + - Ensure consistent naming |
| 265 | + - Validate documentation |
| 266 | + - Review for best practices |
| 267 | + |
| 268 | +3. **Style Guide Resources** |
| 269 | + - [Google Python Style Guide](https://google.github.io/styleguide/pyguide.html) |
| 270 | + |
| 271 | +--- |
| 272 | + |
| 273 | +Remember: These guidelines are living documents. Feel free to suggest improvements and updates as the team evolves and learns. |
0 commit comments