|
| 1 | +### 1. Avoid Breaking Changes (Stable Public Interfaces) |
| 2 | + |
| 3 | +* Carefully preserve **function signatures**, argument positions, and names for any exported/public methods. |
| 4 | +* Be cautious when **renaming**, **removing**, or **reordering** arguments — even small changes can break downstream consumers. |
| 5 | +* Use keyword-only arguments or clearly mark experimental features to isolate unstable APIs. |
| 6 | + |
| 7 | +Bad: |
| 8 | + |
| 9 | +```python |
| 10 | +def get_user(id, verbose=False): # Changed from `user_id` |
| 11 | +``` |
| 12 | + |
| 13 | +Good: |
| 14 | + |
| 15 | +```python |
| 16 | +def get_user(user_id: str, verbose: bool = False): # Maintains stable interface |
| 17 | +``` |
| 18 | + |
| 19 | +🧠 *Ask yourself:* “Would this change break someone's code if they used it last week?” |
| 20 | + |
| 21 | +--- |
| 22 | + |
| 23 | +### 2. Simplify Code and Use Clear Variable Names |
| 24 | + |
| 25 | +* Prefer descriptive, **self-explanatory variable names**. Avoid overly short or cryptic identifiers. |
| 26 | +* Break up overly long or deeply nested functions for **readability and maintainability**. |
| 27 | +* Avoid unnecessary abstraction or premature optimization. |
| 28 | + |
| 29 | +Bad: |
| 30 | + |
| 31 | +```python |
| 32 | +def p(u, d): |
| 33 | + return [x for x in u if x not in d] |
| 34 | +``` |
| 35 | + |
| 36 | +Good: |
| 37 | + |
| 38 | +```python |
| 39 | +def filter_unknown_users(users: List[str], known_users: Set[str]) -> List[str]: |
| 40 | + return [user for user in users if user not in known_users] |
| 41 | +``` |
| 42 | + |
| 43 | +--- |
| 44 | + |
| 45 | +### 3. Ensure Unit Tests Cover New and Updated Functionality |
| 46 | + |
| 47 | +* Every new feature or bugfix should be **covered by a unit test**. |
| 48 | +* Test edge cases and failure conditions. |
| 49 | +* Use `pytest`, `unittest`, or the project’s existing framework consistently. |
| 50 | + |
| 51 | +Checklist: |
| 52 | + |
| 53 | +* [ ] Does the test suite fail if your new logic is broken? |
| 54 | +* [ ] Are all expected behaviors exercised (happy path, invalid input, etc)? |
| 55 | +* [ ] Do tests use fixtures or mocks where needed? |
| 56 | + |
| 57 | +--- |
| 58 | + |
| 59 | +### 4. Look for Suspicious or Risky Code |
| 60 | + |
| 61 | +* Watch out for: |
| 62 | + |
| 63 | + * Use of `eval()`, `exec()`, or `pickle` on user-controlled input. |
| 64 | + * Silent failure modes (`except: pass`). |
| 65 | + * Unreachable code or commented-out blocks. |
| 66 | + * Race conditions or resource leaks (file handles, sockets, threads). |
| 67 | + |
| 68 | +Bad: |
| 69 | + |
| 70 | +```python |
| 71 | +def load_config(path): |
| 72 | + with open(path) as f: |
| 73 | + return eval(f.read()) # ⚠️ Never eval config |
| 74 | +``` |
| 75 | + |
| 76 | +Good: |
| 77 | + |
| 78 | +```python |
| 79 | +import json |
| 80 | + |
| 81 | +def load_config(path: str) -> dict: |
| 82 | + with open(path) as f: |
| 83 | + return json.load(f) |
| 84 | +``` |
| 85 | + |
| 86 | +--- |
| 87 | + |
| 88 | +### 5. Use Google-Style Docstrings (with Args section) |
| 89 | + |
| 90 | +* All public functions should include a **Google-style docstring**. |
| 91 | +* Include an `Args:` section where relevant. |
| 92 | +* Types should NOT be written in the docstring — use type hints instead. |
| 93 | + |
| 94 | +Bad: |
| 95 | + |
| 96 | +```python |
| 97 | +def send_email(to, msg): |
| 98 | + """Send an email to a recipient.""" |
| 99 | +``` |
| 100 | + |
| 101 | +Good: |
| 102 | + |
| 103 | +```python |
| 104 | +def send_email(to: str, msg: str) -> None: |
| 105 | + """ |
| 106 | + Sends an email to a recipient. |
| 107 | +
|
| 108 | + Args: |
| 109 | + to: The email address of the recipient. |
| 110 | + msg: The message body. |
| 111 | + """ |
| 112 | +``` |
| 113 | + |
| 114 | +📌 *Tip:* Keep descriptions concise but clear. Only document return values if non-obvious. |
| 115 | + |
| 116 | +--- |
| 117 | + |
| 118 | +### 6. Propose Better Designs When Applicable |
| 119 | + |
| 120 | +* If there's a **cleaner**, **more scalable**, or **simpler** design, highlight it. |
| 121 | +* Suggest improvements, even if they require some refactoring — especially if the new code would: |
| 122 | + |
| 123 | + * Reduce duplication |
| 124 | + * Make unit testing easier |
| 125 | + * Improve separation of concerns |
| 126 | + * Add clarity without adding complexity |
| 127 | + |
| 128 | +Instead of: |
| 129 | + |
| 130 | +```python |
| 131 | +def save(data, db_conn): |
| 132 | + # manually serializes fields |
| 133 | +``` |
| 134 | + |
| 135 | +You might suggest: |
| 136 | + |
| 137 | +```python |
| 138 | +# Suggest using dataclasses or Pydantic for automatic serialization and validation |
| 139 | +``` |
0 commit comments