Add comprehensive development and style guides for Elytra PIM Client
- Introduced .copilot-instructions.md for GitHub Copilot usage guidelines. - Created DEVELOPMENT.md detailing environment setup, workflow, and common tasks. - Established STYLE_GUIDE.md as a quick reference for code style, formatting, and conventions.
This commit is contained in:
parent
05fca294f9
commit
459838b2e6
4 changed files with 1794 additions and 0 deletions
590
STYLE_GUIDE.md
Normal file
590
STYLE_GUIDE.md
Normal file
|
|
@ -0,0 +1,590 @@
|
|||
# Code Style Guide - Quick Reference
|
||||
|
||||
## Language: ENGLISH ONLY
|
||||
|
||||
All code, comments, docstrings, and variable names **MUST** be in English.
|
||||
|
||||
---
|
||||
|
||||
## Formatting
|
||||
|
||||
### Line Length
|
||||
- **Maximum: 100 characters**
|
||||
- Configure editor ruler: `View → Toggle Rulers`
|
||||
|
||||
### Indentation
|
||||
- **4 spaces** (no tabs)
|
||||
- Auto-enforced by Black
|
||||
|
||||
### Blank Lines
|
||||
- **2 blank lines** between module-level definitions
|
||||
- **1 blank line** between class methods
|
||||
- **1 blank line** inside functions for logical grouping
|
||||
|
||||
---
|
||||
|
||||
## Imports
|
||||
|
||||
### Order
|
||||
```python
|
||||
# 1. Standard library
|
||||
import sys
|
||||
import os
|
||||
from pathlib import Path
|
||||
from typing import Optional, Dict, List, TypeVar, Type
|
||||
|
||||
# 2. Third-party
|
||||
import requests
|
||||
from pydantic import BaseModel, Field, ValidationError
|
||||
from dotenv import load_dotenv
|
||||
|
||||
# 3. Local/relative
|
||||
from .exceptions import ElytraAPIError
|
||||
from .models import ProductResponse
|
||||
```
|
||||
|
||||
### Rules
|
||||
- ✅ Use absolute imports
|
||||
- ✅ Group by: stdlib → third-party → local
|
||||
- ✅ Alphabetical within groups
|
||||
- ❌ No `from module import *` (use explicit imports)
|
||||
- ❌ No unused imports (checked by flake8)
|
||||
|
||||
---
|
||||
|
||||
## Type Hints
|
||||
|
||||
### Functions
|
||||
```python
|
||||
# Good
|
||||
def get_product(
|
||||
product_id: str,
|
||||
lang: str = "en"
|
||||
) -> ProductResponse:
|
||||
"""Get a product."""
|
||||
pass
|
||||
|
||||
# Good - with Optional
|
||||
def find_product(
|
||||
name: Optional[str] = None
|
||||
) -> Optional[ProductResponse]:
|
||||
"""Find product by name."""
|
||||
pass
|
||||
|
||||
# Good - with Union (Python 3.10+)
|
||||
def process(data: dict | list) -> str:
|
||||
"""Process data."""
|
||||
pass
|
||||
|
||||
# Bad - missing type hints
|
||||
def get_products(product_id):
|
||||
return response
|
||||
```
|
||||
|
||||
### Variables
|
||||
```python
|
||||
# At module level
|
||||
API_TIMEOUT: int = 30
|
||||
DEFAULT_LANG: str = "en"
|
||||
ALLOWED_LANGS: List[str] = ["en", "de", "fr"]
|
||||
|
||||
# In functions
|
||||
active_products: List[Product] = []
|
||||
product_map: Dict[str, Product] = {}
|
||||
result: Optional[ProductResponse] = None
|
||||
```
|
||||
|
||||
### Generic Types
|
||||
```python
|
||||
from typing import TypeVar, Generic, List
|
||||
|
||||
T = TypeVar('T', bound=BaseModel)
|
||||
|
||||
def validate_response(
|
||||
data: dict,
|
||||
model: Type[T]
|
||||
) -> T:
|
||||
"""Generic response validator."""
|
||||
return model.model_validate(data)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Docstrings (Google Style)
|
||||
|
||||
### Module Level
|
||||
```python
|
||||
"""Module description.
|
||||
|
||||
Longer explanation if needed.
|
||||
|
||||
This module handles product management operations including
|
||||
creating, updating, and retrieving products from the API.
|
||||
"""
|
||||
```
|
||||
|
||||
### Classes
|
||||
```python
|
||||
class ElytraClient:
|
||||
"""A Pythonic client for the Elytra PIM API.
|
||||
|
||||
This client provides convenient methods for interacting with
|
||||
the Elytra PIM API, including authentication and product management.
|
||||
|
||||
Args:
|
||||
base_url: The base URL of the Elytra PIM API
|
||||
api_key: The API key for authentication
|
||||
timeout: Request timeout in seconds (default: 30)
|
||||
|
||||
Attributes:
|
||||
session: Requests.Session instance for connection pooling
|
||||
"""
|
||||
pass
|
||||
```
|
||||
|
||||
### Functions/Methods
|
||||
```python
|
||||
def create_product(
|
||||
self,
|
||||
name: str,
|
||||
description: str,
|
||||
lang: str = "en"
|
||||
) -> SingleProductResponse:
|
||||
"""Create a new product.
|
||||
|
||||
Creates a new product in the Elytra PIM system with the provided
|
||||
information.
|
||||
|
||||
Args:
|
||||
name: Product name (required, max 255 chars)
|
||||
description: Product description
|
||||
lang: Language code (default: "en")
|
||||
|
||||
Returns:
|
||||
SingleProductResponse: The newly created product with ID
|
||||
|
||||
Raises:
|
||||
ElytraValidationError: If validation fails
|
||||
ElytraAuthenticationError: If authentication fails
|
||||
ElytraAPIError: For other API errors
|
||||
|
||||
Example:
|
||||
>>> client = ElytraClient(base_url="...", api_key="...")
|
||||
>>> product = client.create_product("Widget", "A useful widget")
|
||||
>>> print(product.id)
|
||||
"""
|
||||
pass
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Naming Conventions
|
||||
|
||||
### Constants
|
||||
```python
|
||||
# UPPER_CASE_WITH_UNDERSCORES
|
||||
API_TIMEOUT = 30
|
||||
MAX_RETRIES = 3
|
||||
DEFAULT_LANGUAGE = "en"
|
||||
```
|
||||
|
||||
### Classes
|
||||
```python
|
||||
# PascalCase
|
||||
class ElytraClient:
|
||||
pass
|
||||
|
||||
class ProductResponse:
|
||||
pass
|
||||
|
||||
class InvalidProductError:
|
||||
pass
|
||||
```
|
||||
|
||||
### Functions & Methods
|
||||
```python
|
||||
# snake_case
|
||||
def get_products():
|
||||
pass
|
||||
|
||||
def create_new_product():
|
||||
pass
|
||||
|
||||
def validate_input():
|
||||
pass
|
||||
```
|
||||
|
||||
### Variables
|
||||
```python
|
||||
# snake_case
|
||||
product_id = "123"
|
||||
api_key = "secret"
|
||||
response_data = {}
|
||||
is_valid = True
|
||||
```
|
||||
|
||||
### Private Methods/Attributes
|
||||
```python
|
||||
# Leading underscore for private
|
||||
def _make_request(self):
|
||||
pass
|
||||
|
||||
def _validate_response(self, response):
|
||||
pass
|
||||
|
||||
_cache = {}
|
||||
_internal_state = None
|
||||
```
|
||||
|
||||
### Constants vs Variables
|
||||
```python
|
||||
# Constants (module-level, unchanging)
|
||||
DEFAULT_TIMEOUT = 30
|
||||
API_VERSION = "v1"
|
||||
|
||||
# Variables (local, changeable)
|
||||
timeout = config.get("timeout", DEFAULT_TIMEOUT)
|
||||
request_id = str(uuid4())
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Strings
|
||||
|
||||
### Formatting
|
||||
```python
|
||||
# f-strings (Python 3.6+) - PREFERRED
|
||||
name = "Product"
|
||||
print(f"Created product: {name}")
|
||||
|
||||
# Format method
|
||||
print("Created product: {}".format(name))
|
||||
|
||||
# % formatting - AVOID
|
||||
print("Created product: %s" % name)
|
||||
```
|
||||
|
||||
### Quotes
|
||||
```python
|
||||
# Use double quotes as default
|
||||
message = "This is a message"
|
||||
docstring = """Multi-line docstring used with three
|
||||
double quotes."""
|
||||
|
||||
# Use single quotes when string contains double quote
|
||||
message = 'He said "Hello"'
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Exceptions
|
||||
|
||||
### Handling
|
||||
```python
|
||||
# Good - specific exception, with context
|
||||
try:
|
||||
response = self.session.request(method, url)
|
||||
response.raise_for_status()
|
||||
except requests.HTTPError as e:
|
||||
if e.response.status_code == 404:
|
||||
raise ElytraNotFoundError("Resource not found") from e
|
||||
raise ElytraAPIError(f"HTTP Error: {e}") from e
|
||||
except requests.RequestException as e:
|
||||
raise ElytraAPIError(f"Network error: {str(e)}") from e
|
||||
|
||||
# Bad - too broad, no context
|
||||
try:
|
||||
response = requests.get(url)
|
||||
except:
|
||||
print("Error")
|
||||
|
||||
# Bad - bare raise without context
|
||||
except Exception:
|
||||
return None
|
||||
```
|
||||
|
||||
### Raising
|
||||
```python
|
||||
# Good - with meaningful message
|
||||
raise ElytraValidationError(f"Invalid product ID: {product_id}")
|
||||
|
||||
# Good - with cause chain
|
||||
try:
|
||||
data = pydantic_model.model_validate(response)
|
||||
except ValidationError as e:
|
||||
raise ElytraValidationError(f"Response validation failed") from e
|
||||
|
||||
# Bad - generic message
|
||||
raise ElytraAPIError("Error")
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Pydantic Models
|
||||
|
||||
```python
|
||||
from pydantic import BaseModel, Field, ConfigDict
|
||||
|
||||
class ProductResponse(BaseModel):
|
||||
"""Response model for a single product.
|
||||
|
||||
Validates and represents a product object from the API.
|
||||
"""
|
||||
# Required field with description
|
||||
id: str = Field(..., description="Unique product identifier")
|
||||
name: str = Field(..., description="Product name", min_length=1, max_length=255)
|
||||
|
||||
# Optional field with default
|
||||
description: Optional[str] = Field(None, description="Product description")
|
||||
|
||||
# Field with default value
|
||||
language: str = Field("en", description="Language code")
|
||||
|
||||
# List field with default factory
|
||||
tags: List[str] = Field(default_factory=list, description="Product tags")
|
||||
|
||||
# Configuration
|
||||
model_config = ConfigDict(
|
||||
str_strip_whitespace=True, # Automatically strip whitespace from strings
|
||||
validate_default=True, # Validate default values
|
||||
)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Comments
|
||||
|
||||
### Good Comments
|
||||
```python
|
||||
# Good - explains WHY, not WHAT
|
||||
# We need to retry failed requests as the API has rate limiting
|
||||
for retry in range(MAX_RETRIES):
|
||||
try:
|
||||
return self._make_request(...)
|
||||
except RequestException:
|
||||
if retry == MAX_RETRIES - 1:
|
||||
raise
|
||||
|
||||
# Good - documents non-obvious code
|
||||
# Using session instead of individual requests for connection pooling
|
||||
self.session = requests.Session()
|
||||
|
||||
# Good - TODO comments
|
||||
# TODO: implement pagination for large datasets (issue #123)
|
||||
def get_all_products(self):
|
||||
pass
|
||||
```
|
||||
|
||||
### Bad Comments
|
||||
```python
|
||||
# Bad - obvious, doesn't add value
|
||||
# Increment counter
|
||||
counter += 1
|
||||
|
||||
# Bad - outdated/wrong info
|
||||
# This will be removed next release (said in 2020)
|
||||
def deprecated_method(self):
|
||||
pass
|
||||
|
||||
# BAD - NEVER DO THIS
|
||||
# código de prueba
|
||||
# 这个是测试代码
|
||||
# Ceci est un test
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Control Flow
|
||||
|
||||
### If Statements
|
||||
```python
|
||||
# Good - clear and readable
|
||||
if user_is_authenticated and not is_rate_limited:
|
||||
return process_request(data)
|
||||
|
||||
# Bad - too complex
|
||||
if x and y or z and not w:
|
||||
pass
|
||||
|
||||
# Good - use `in` for membership
|
||||
if method in ("GET", "POST", "PUT"):
|
||||
pass
|
||||
|
||||
# Good - use `is` for None, True, False
|
||||
if result is None:
|
||||
pass
|
||||
|
||||
if is_valid is True: # or just: if is_valid:
|
||||
pass
|
||||
```
|
||||
|
||||
### Loops
|
||||
```python
|
||||
# Good - enumerate for index and value
|
||||
for idx, item in enumerate(items):
|
||||
print(f"{idx}: {item}")
|
||||
|
||||
# Good - dict iteration
|
||||
for key, value in config.items():
|
||||
print(f"{key} = {value}")
|
||||
|
||||
# Avoid - unnecessary else clause
|
||||
for item in items:
|
||||
if match(item):
|
||||
break
|
||||
else:
|
||||
# Only executes if loop completed without break
|
||||
# Often confusing - better to use explicit flag
|
||||
handle_not_found()
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Context Managers
|
||||
|
||||
```python
|
||||
# Good - automatic cleanup
|
||||
with session.get(url) as response:
|
||||
data = response.json()
|
||||
|
||||
# For custom classes
|
||||
class ElytraClient:
|
||||
def __enter__(self):
|
||||
"""Enter context."""
|
||||
return self
|
||||
|
||||
def __exit__(self, exc_type, exc_val, exc_tb):
|
||||
"""Exit context and cleanup."""
|
||||
self.session.close()
|
||||
return False # Don't suppress exceptions
|
||||
|
||||
# Usage
|
||||
with ElytraClient(base_url, api_key) as client:
|
||||
products = client.get_products()
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Whitespace and Blank Lines
|
||||
|
||||
### Module Structure
|
||||
```python
|
||||
"""Module docstring."""
|
||||
|
||||
# Imports (with blank line after)
|
||||
import os
|
||||
from typing import List
|
||||
|
||||
# Module constants (with blank line before and after)
|
||||
DEFAULT_TIMEOUT = 30
|
||||
API_VERSION = "v1"
|
||||
|
||||
# Classes (with 2 blank lines before)
|
||||
|
||||
|
||||
class FirstClass:
|
||||
"""Class docstring."""
|
||||
|
||||
def method_one(self):
|
||||
"""First method."""
|
||||
pass
|
||||
|
||||
def method_two(self):
|
||||
"""Second method."""
|
||||
pass
|
||||
|
||||
|
||||
class SecondClass:
|
||||
"""Another class."""
|
||||
pass
|
||||
|
||||
|
||||
# Module-level functions (with 2 blank lines before)
|
||||
|
||||
|
||||
def module_function():
|
||||
"""A module-level function."""
|
||||
pass
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Line Breaking
|
||||
|
||||
```python
|
||||
# Good - break long imports
|
||||
from pydantic import (
|
||||
BaseModel,
|
||||
Field,
|
||||
ValidationError,
|
||||
ConfigDict,
|
||||
)
|
||||
|
||||
# Good - break long function signatures
|
||||
def create_product(
|
||||
self,
|
||||
name: str,
|
||||
description: Optional[str] = None,
|
||||
lang: str = "en",
|
||||
) -> SingleProductResponse:
|
||||
pass
|
||||
|
||||
# Good - break long lines with backslash (function calls)
|
||||
result = self._make_request(
|
||||
method="POST",
|
||||
endpoint="/products",
|
||||
json_data=product_data,
|
||||
response_model=ProductResponse,
|
||||
)
|
||||
|
||||
# Good - break long conditionals
|
||||
if (
|
||||
user_is_authenticated and
|
||||
not is_rate_limited and
|
||||
has_valid_request_data
|
||||
):
|
||||
process_request()
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Linting and Formatting Commands
|
||||
|
||||
```bash
|
||||
# Format with Black
|
||||
black elytra_client tests
|
||||
|
||||
# Sort imports with isort
|
||||
isort elytra_client tests
|
||||
|
||||
# Lint with Flake8
|
||||
flake8 elytra_client tests
|
||||
|
||||
# Type check with mypy
|
||||
mypy elytra_client
|
||||
|
||||
# Run all (recommended before commit)
|
||||
black elytra_client tests && \
|
||||
isort elytra_client tests && \
|
||||
flake8 elytra_client tests && \
|
||||
mypy elytra_client && \
|
||||
pytest tests/ -v
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Summary Checklist
|
||||
|
||||
Before committing code:
|
||||
|
||||
- [ ] All code is in **English**
|
||||
- [ ] **No lines exceed 100 characters** (Black will enforce)
|
||||
- [ ] **Type hints** on all functions
|
||||
- [ ] **Docstrings** on all public APIs (Google style)
|
||||
- [ ] **No unused imports** (Flake8 checks)
|
||||
- [ ] **Proper exception handling** with meaningful messages
|
||||
- [ ] **Tests** for new code
|
||||
- [ ] Run Black and isort
|
||||
- [ ] Run Flake8
|
||||
- [ ] Run mypy
|
||||
- [ ] Run pytest
|
||||
Loading…
Add table
Add a link
Reference in a new issue