added more files
This commit is contained in:
33
.github/copilot-instructions.md
vendored
Normal file
33
.github/copilot-instructions.md
vendored
Normal file
@@ -0,0 +1,33 @@
|
||||
---
|
||||
applyTo: "**"
|
||||
---
|
||||
## Project general
|
||||
- Digital Read Out for a lathe
|
||||
- Uses 2 rotary encoders to keep track of lathe tool location
|
||||
- Code base is a mix of C++ Arduino code and python
|
||||
- The main python application is deployed on Raspberry Pi
|
||||
- Architectural pattern, the main application uses MVC
|
||||
- the main application reads rotary encoder location from Arduino via i2c
|
||||
- Documentation standards: Use clear and concise language, include code examples where applicable, and maintain consistent formatting throughout the documentation.
|
||||
- Try to follow the DRY and KISS principles
|
||||
|
||||
## Project structure
|
||||
|
||||
- i2c_encoder/ : arduino application for reading encoder position and transitting via i2c
|
||||
RPi/ : fake python module for testing on PC
|
||||
|
||||
## Tech stack in use
|
||||
|
||||
- Raspberry Pi 4
|
||||
- Tkinter front-end
|
||||
- Arduino Nano
|
||||
|
||||
## Custom instructions
|
||||
|
||||
| Instruction | Description |
|
||||
|-------------|-------------|
|
||||
|[Code review](./instructions/code-review-generic.instructions.md) | Generic code review instructions that can be customized for any project using GitHub Copilot |
|
||||
|[C++](./instructions/cpp.instructions.md) | Language specific instructions for C++ |
|
||||
| [Python](./instructions/python.instructions.md) | Language specific instructions for Python |
|
||||
| [Documentation](./instructions/documentation.instructions.md) | Generall instructions when writing documentation |
|
||||
| [Shell](./instructions/shell.instructions.md) | Generall instructions when writing shell scripts |
|
||||
418
.github/instructions/code-review-generic.instructions.md
vendored
Normal file
418
.github/instructions/code-review-generic.instructions.md
vendored
Normal file
@@ -0,0 +1,418 @@
|
||||
---
|
||||
description: 'Generic code review instructions that can be customized for any project using GitHub Copilot'
|
||||
applyTo: '**'
|
||||
excludeAgent: ["coding-agent"]
|
||||
---
|
||||
|
||||
# Generic Code Review Instructions
|
||||
|
||||
Comprehensive code review guidelines for GitHub Copilot that can be adapted to any project. These instructions follow best practices from prompt engineering and provide a structured approach to code quality, security, testing, and architecture review.
|
||||
|
||||
## Review Language
|
||||
|
||||
When performing a code review, respond in **English** (or specify your preferred language).
|
||||
|
||||
> **Customization Tip**: Change to your preferred language by replacing "English" with "Portuguese (Brazilian)", "Spanish", "French", etc.
|
||||
|
||||
## Review Priorities
|
||||
|
||||
When performing a code review, prioritize issues in the following order:
|
||||
|
||||
### 🔴 CRITICAL (Block merge)
|
||||
- **Security**: Vulnerabilities, exposed secrets, authentication/authorization issues
|
||||
- **Correctness**: Logic errors, data corruption risks, race conditions
|
||||
- **Breaking Changes**: API contract changes without versioning
|
||||
- **Data Loss**: Risk of data loss or corruption
|
||||
|
||||
### 🟡 IMPORTANT (Requires discussion)
|
||||
- **Code Quality**: Severe violations of SOLID principles, excessive duplication
|
||||
- **Test Coverage**: Missing tests for critical paths or new functionality
|
||||
- **Performance**: Obvious performance bottlenecks (N+1 queries, memory leaks)
|
||||
- **Architecture**: Significant deviations from established patterns
|
||||
|
||||
### 🟢 SUGGESTION (Non-blocking improvements)
|
||||
- **Readability**: Poor naming, complex logic that could be simplified
|
||||
- **Optimization**: Performance improvements without functional impact
|
||||
- **Best Practices**: Minor deviations from conventions
|
||||
- **Documentation**: Missing or incomplete comments/documentation
|
||||
|
||||
## General Review Principles
|
||||
|
||||
When performing a code review, follow these principles:
|
||||
|
||||
1. **Be specific**: Reference exact lines, files, and provide concrete examples
|
||||
2. **Provide context**: Explain WHY something is an issue and the potential impact
|
||||
3. **Suggest solutions**: Show corrected code when applicable, not just what's wrong
|
||||
4. **Be constructive**: Focus on improving the code, not criticizing the author
|
||||
5. **Recognize good practices**: Acknowledge well-written code and smart solutions
|
||||
6. **Be pragmatic**: Not every suggestion needs immediate implementation
|
||||
7. **Group related comments**: Avoid multiple comments about the same topic
|
||||
|
||||
## Code Quality Standards
|
||||
|
||||
When performing a code review, check for:
|
||||
|
||||
### Clean Code
|
||||
- Descriptive and meaningful names for variables, functions, and classes
|
||||
- Single Responsibility Principle: each function/class does one thing well
|
||||
- DRY (Don't Repeat Yourself): no code duplication
|
||||
- Functions should be small and focused (ideally < 20-30 lines)
|
||||
- Avoid deeply nested code (max 3-4 levels)
|
||||
- Avoid magic numbers and strings (use constants)
|
||||
- Code should be self-documenting; comments only when necessary
|
||||
|
||||
### Examples
|
||||
```javascript
|
||||
// ❌ BAD: Poor naming and magic numbers
|
||||
function calc(x, y) {
|
||||
if (x > 100) return y * 0.15;
|
||||
return y * 0.10;
|
||||
}
|
||||
|
||||
// ✅ GOOD: Clear naming and constants
|
||||
const PREMIUM_THRESHOLD = 100;
|
||||
const PREMIUM_DISCOUNT_RATE = 0.15;
|
||||
const STANDARD_DISCOUNT_RATE = 0.10;
|
||||
|
||||
function calculateDiscount(orderTotal, itemPrice) {
|
||||
const isPremiumOrder = orderTotal > PREMIUM_THRESHOLD;
|
||||
const discountRate = isPremiumOrder ? PREMIUM_DISCOUNT_RATE : STANDARD_DISCOUNT_RATE;
|
||||
return itemPrice * discountRate;
|
||||
}
|
||||
```
|
||||
|
||||
### Error Handling
|
||||
- Proper error handling at appropriate levels
|
||||
- Meaningful error messages
|
||||
- No silent failures or ignored exceptions
|
||||
- Fail fast: validate inputs early
|
||||
- Use appropriate error types/exceptions
|
||||
|
||||
### Examples
|
||||
```python
|
||||
# ❌ BAD: Silent failure and generic error
|
||||
def process_user(user_id):
|
||||
try:
|
||||
user = db.get(user_id)
|
||||
user.process()
|
||||
except:
|
||||
pass
|
||||
|
||||
# ✅ GOOD: Explicit error handling
|
||||
def process_user(user_id):
|
||||
if not user_id or user_id <= 0:
|
||||
raise ValueError(f"Invalid user_id: {user_id}")
|
||||
|
||||
try:
|
||||
user = db.get(user_id)
|
||||
except UserNotFoundError:
|
||||
raise UserNotFoundError(f"User {user_id} not found in database")
|
||||
except DatabaseError as e:
|
||||
raise ProcessingError(f"Failed to retrieve user {user_id}: {e}")
|
||||
|
||||
return user.process()
|
||||
```
|
||||
|
||||
## Security Review
|
||||
|
||||
When performing a code review, check for security issues:
|
||||
|
||||
- **Sensitive Data**: No passwords, API keys, tokens, or PII in code or logs
|
||||
- **Input Validation**: All user inputs are validated and sanitized
|
||||
- **SQL Injection**: Use parameterized queries, never string concatenation
|
||||
- **Authentication**: Proper authentication checks before accessing resources
|
||||
- **Authorization**: Verify user has permission to perform action
|
||||
- **Cryptography**: Use established libraries, never roll your own crypto
|
||||
- **Dependency Security**: Check for known vulnerabilities in dependencies
|
||||
|
||||
### Examples
|
||||
```java
|
||||
// ❌ BAD: SQL injection vulnerability
|
||||
String query = "SELECT * FROM users WHERE email = '" + email + "'";
|
||||
|
||||
// ✅ GOOD: Parameterized query
|
||||
PreparedStatement stmt = conn.prepareStatement(
|
||||
"SELECT * FROM users WHERE email = ?"
|
||||
);
|
||||
stmt.setString(1, email);
|
||||
```
|
||||
|
||||
```javascript
|
||||
// ❌ BAD: Exposed secret in code
|
||||
const API_KEY = "sk_live_abc123xyz789";
|
||||
|
||||
// ✅ GOOD: Use environment variables
|
||||
const API_KEY = process.env.API_KEY;
|
||||
```
|
||||
|
||||
## Testing Standards
|
||||
|
||||
When performing a code review, verify test quality:
|
||||
|
||||
- **Coverage**: Critical paths and new functionality must have tests
|
||||
- **Test Names**: Descriptive names that explain what is being tested
|
||||
- **Test Structure**: Clear Arrange-Act-Assert or Given-When-Then pattern
|
||||
- **Independence**: Tests should not depend on each other or external state
|
||||
- **Assertions**: Use specific assertions, avoid generic assertTrue/assertFalse
|
||||
- **Edge Cases**: Test boundary conditions, null values, empty collections
|
||||
- **Mock Appropriately**: Mock external dependencies, not domain logic
|
||||
|
||||
### Examples
|
||||
```typescript
|
||||
// ❌ BAD: Vague name and assertion
|
||||
test('test1', () => {
|
||||
const result = calc(5, 10);
|
||||
expect(result).toBeTruthy();
|
||||
});
|
||||
|
||||
// ✅ GOOD: Descriptive name and specific assertion
|
||||
test('should calculate 10% discount for orders under $100', () => {
|
||||
const orderTotal = 50;
|
||||
const itemPrice = 20;
|
||||
|
||||
const discount = calculateDiscount(orderTotal, itemPrice);
|
||||
|
||||
expect(discount).toBe(2.00);
|
||||
});
|
||||
```
|
||||
|
||||
## Performance Considerations
|
||||
|
||||
When performing a code review, check for performance issues:
|
||||
|
||||
- **Database Queries**: Avoid N+1 queries, use proper indexing
|
||||
- **Algorithms**: Appropriate time/space complexity for the use case
|
||||
- **Caching**: Utilize caching for expensive or repeated operations
|
||||
- **Resource Management**: Proper cleanup of connections, files, streams
|
||||
- **Pagination**: Large result sets should be paginated
|
||||
- **Lazy Loading**: Load data only when needed
|
||||
|
||||
### Examples
|
||||
```python
|
||||
# ❌ BAD: N+1 query problem
|
||||
users = User.query.all()
|
||||
for user in users:
|
||||
orders = Order.query.filter_by(user_id=user.id).all() # N+1!
|
||||
|
||||
# ✅ GOOD: Use JOIN or eager loading
|
||||
users = User.query.options(joinedload(User.orders)).all()
|
||||
for user in users:
|
||||
orders = user.orders
|
||||
```
|
||||
|
||||
## Architecture and Design
|
||||
|
||||
When performing a code review, verify architectural principles:
|
||||
|
||||
- **Separation of Concerns**: Clear boundaries between layers/modules
|
||||
- **Dependency Direction**: High-level modules don't depend on low-level details
|
||||
- **Interface Segregation**: Prefer small, focused interfaces
|
||||
- **Loose Coupling**: Components should be independently testable
|
||||
- **High Cohesion**: Related functionality grouped together
|
||||
- **Consistent Patterns**: Follow established patterns in the codebase
|
||||
|
||||
## Documentation Standards
|
||||
|
||||
When performing a code review, check documentation:
|
||||
|
||||
- **API Documentation**: Public APIs must be documented (purpose, parameters, returns)
|
||||
- **Complex Logic**: Non-obvious logic should have explanatory comments
|
||||
- **README Updates**: Update README when adding features or changing setup
|
||||
- **Breaking Changes**: Document any breaking changes clearly
|
||||
- **Examples**: Provide usage examples for complex features
|
||||
|
||||
## Comment Format Template
|
||||
|
||||
When performing a code review, use this format for comments:
|
||||
|
||||
```markdown
|
||||
**[PRIORITY] Category: Brief title**
|
||||
|
||||
Detailed description of the issue or suggestion.
|
||||
|
||||
**Why this matters:**
|
||||
Explanation of the impact or reason for the suggestion.
|
||||
|
||||
**Suggested fix:**
|
||||
[code example if applicable]
|
||||
|
||||
**Reference:** [link to relevant documentation or standard]
|
||||
```
|
||||
|
||||
### Example Comments
|
||||
|
||||
#### Critical Issue
|
||||
````markdown
|
||||
**🔴 CRITICAL - Security: SQL Injection Vulnerability**
|
||||
|
||||
The query on line 45 concatenates user input directly into the SQL string,
|
||||
creating a SQL injection vulnerability.
|
||||
|
||||
**Why this matters:**
|
||||
An attacker could manipulate the email parameter to execute arbitrary SQL commands,
|
||||
potentially exposing or deleting all database data.
|
||||
|
||||
**Suggested fix:**
|
||||
```sql
|
||||
-- Instead of:
|
||||
query = "SELECT * FROM users WHERE email = '" + email + "'"
|
||||
|
||||
-- Use:
|
||||
PreparedStatement stmt = conn.prepareStatement(
|
||||
"SELECT * FROM users WHERE email = ?"
|
||||
);
|
||||
stmt.setString(1, email);
|
||||
```
|
||||
|
||||
**Reference:** OWASP SQL Injection Prevention Cheat Sheet
|
||||
````
|
||||
|
||||
#### Important Issue
|
||||
````markdown
|
||||
**🟡 IMPORTANT - Testing: Missing test coverage for critical path**
|
||||
|
||||
The `processPayment()` function handles financial transactions but has no tests
|
||||
for the refund scenario.
|
||||
|
||||
**Why this matters:**
|
||||
Refunds involve money movement and should be thoroughly tested to prevent
|
||||
financial errors or data inconsistencies.
|
||||
|
||||
**Suggested fix:**
|
||||
Add test case:
|
||||
```javascript
|
||||
test('should process full refund when order is cancelled', () => {
|
||||
const order = createOrder({ total: 100, status: 'cancelled' });
|
||||
|
||||
const result = processPayment(order, { type: 'refund' });
|
||||
|
||||
expect(result.refundAmount).toBe(100);
|
||||
expect(result.status).toBe('refunded');
|
||||
});
|
||||
```
|
||||
````
|
||||
|
||||
#### Suggestion
|
||||
````markdown
|
||||
**🟢 SUGGESTION - Readability: Simplify nested conditionals**
|
||||
|
||||
The nested if statements on lines 30-40 make the logic hard to follow.
|
||||
|
||||
**Why this matters:**
|
||||
Simpler code is easier to maintain, debug, and test.
|
||||
|
||||
**Suggested fix:**
|
||||
```javascript
|
||||
// Instead of nested ifs:
|
||||
if (user) {
|
||||
if (user.isActive) {
|
||||
if (user.hasPermission('write')) {
|
||||
// do something
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Consider guard clauses:
|
||||
if (!user || !user.isActive || !user.hasPermission('write')) {
|
||||
return;
|
||||
}
|
||||
// do something
|
||||
```
|
||||
````
|
||||
|
||||
## Review Checklist
|
||||
|
||||
When performing a code review, systematically verify:
|
||||
|
||||
### Code Quality
|
||||
- [ ] Code follows consistent style and conventions
|
||||
- [ ] Names are descriptive and follow naming conventions
|
||||
- [ ] Functions/methods are small and focused
|
||||
- [ ] No code duplication
|
||||
- [ ] Complex logic is broken into simpler parts
|
||||
- [ ] Error handling is appropriate
|
||||
- [ ] No commented-out code or TODO without tickets
|
||||
|
||||
### Security
|
||||
- [ ] No sensitive data in code or logs
|
||||
- [ ] Input validation on all user inputs
|
||||
- [ ] No SQL injection vulnerabilities
|
||||
- [ ] Authentication and authorization properly implemented
|
||||
- [ ] Dependencies are up-to-date and secure
|
||||
|
||||
### Testing
|
||||
- [ ] New code has appropriate test coverage
|
||||
- [ ] Tests are well-named and focused
|
||||
- [ ] Tests cover edge cases and error scenarios
|
||||
- [ ] Tests are independent and deterministic
|
||||
- [ ] No tests that always pass or are commented out
|
||||
|
||||
### Performance
|
||||
- [ ] No obvious performance issues (N+1, memory leaks)
|
||||
- [ ] Appropriate use of caching
|
||||
- [ ] Efficient algorithms and data structures
|
||||
- [ ] Proper resource cleanup
|
||||
|
||||
### Architecture
|
||||
- [ ] Follows established patterns and conventions
|
||||
- [ ] Proper separation of concerns
|
||||
- [ ] No architectural violations
|
||||
- [ ] Dependencies flow in correct direction
|
||||
|
||||
### Documentation
|
||||
- [ ] Public APIs are documented
|
||||
- [ ] Complex logic has explanatory comments
|
||||
- [ ] README is updated if needed
|
||||
- [ ] Breaking changes are documented
|
||||
|
||||
## Project-Specific Customizations
|
||||
|
||||
To customize this template for your project, add sections for:
|
||||
|
||||
1. **Language/Framework specific checks**
|
||||
- Example: "When performing a code review, verify React hooks follow rules of hooks"
|
||||
- Example: "When performing a code review, check Spring Boot controllers use proper annotations"
|
||||
|
||||
2. **Build and deployment**
|
||||
- Example: "When performing a code review, verify CI/CD pipeline configuration is correct"
|
||||
- Example: "When performing a code review, check database migrations are reversible"
|
||||
|
||||
3. **Business logic rules**
|
||||
- Example: "When performing a code review, verify pricing calculations include all applicable taxes"
|
||||
- Example: "When performing a code review, check user consent is obtained before data processing"
|
||||
|
||||
4. **Team conventions**
|
||||
- Example: "When performing a code review, verify commit messages follow conventional commits format"
|
||||
- Example: "When performing a code review, check branch names follow pattern: type/ticket-description"
|
||||
|
||||
## Additional Resources
|
||||
|
||||
For more information on effective code reviews and GitHub Copilot customization:
|
||||
|
||||
- [GitHub Copilot Prompt Engineering](https://docs.github.com/en/copilot/concepts/prompting/prompt-engineering)
|
||||
- [GitHub Copilot Custom Instructions](https://code.visualstudio.com/docs/copilot/customization/custom-instructions)
|
||||
- [Awesome GitHub Copilot Repository](https://github.com/github/awesome-copilot)
|
||||
- [GitHub Code Review Guidelines](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests)
|
||||
- [Google Engineering Practices - Code Review](https://google.github.io/eng-practices/review/)
|
||||
- [OWASP Security Guidelines](https://owasp.org/)
|
||||
|
||||
## Prompt Engineering Tips
|
||||
|
||||
When performing a code review, apply these prompt engineering principles from the [GitHub Copilot documentation](https://docs.github.com/en/copilot/concepts/prompting/prompt-engineering):
|
||||
|
||||
1. **Start General, Then Get Specific**: Begin with high-level architecture review, then drill into implementation details
|
||||
2. **Give Examples**: Reference similar patterns in the codebase when suggesting changes
|
||||
3. **Break Complex Tasks**: Review large PRs in logical chunks (security → tests → logic → style)
|
||||
4. **Avoid Ambiguity**: Be specific about which file, line, and issue you're addressing
|
||||
5. **Indicate Relevant Code**: Reference related code that might be affected by changes
|
||||
6. **Experiment and Iterate**: If initial review misses something, review again with focused questions
|
||||
|
||||
## Project Context
|
||||
|
||||
This is a generic template. Customize this section with your project-specific information:
|
||||
|
||||
- **Tech Stack**: [e.g., Java 17, Spring Boot 3.x, PostgreSQL]
|
||||
- **Architecture**: [e.g., Hexagonal/Clean Architecture, Microservices]
|
||||
- **Build Tool**: [e.g., Gradle, Maven, npm, pip]
|
||||
- **Testing**: [e.g., JUnit 5, Jest, pytest]
|
||||
- **Code Style**: [e.g., follows Google Style Guide]
|
||||
55
.github/instructions/cpp.instructions.md
vendored
Normal file
55
.github/instructions/cpp.instructions.md
vendored
Normal file
@@ -0,0 +1,55 @@
|
||||
---
|
||||
description: 'Guidelines for building C++ Arduino applications'
|
||||
applyTo: '**/*.cpp, **/*.h, **/*.hpp, **/*ino'
|
||||
---
|
||||
|
||||
# C++ Development
|
||||
|
||||
## C++ Instructions
|
||||
- C++ standard for Arduino.
|
||||
- Write clear and concise comments for each function.
|
||||
|
||||
## General Instructions
|
||||
- Make only high confidence suggestions when reviewing code changes.
|
||||
- Write code with good maintainability practices, including comments on why certain design decisions were made.
|
||||
- Handle edge cases and write clear exception handling.
|
||||
- For libraries or external dependencies, mention their usage and purpose in comments.
|
||||
- Arduino does not support standard libray
|
||||
- Standard library can be used in the unit tests
|
||||
|
||||
## Formatting
|
||||
|
||||
- Apply code-formatting style defined in `.clang-format`.
|
||||
- Prefer file-scoped namespace declarations and single-line using directives.
|
||||
- Insert a newline before the opening curly brace of any code block (e.g., after `if`, `for`, `while`, `foreach`, `using`, `try`, etc.).
|
||||
- Ensure that the final return statement of a method is on its own line.
|
||||
- Use pattern matching and switch expressions wherever possible.
|
||||
- Use `nameof` instead of string literals when referring to member names.
|
||||
- Ensure that XML doc comments are created for any public APIs. When applicable, include `<example>` and `<code>` documentation in the comments.
|
||||
|
||||
## Project Setup and Structure
|
||||
|
||||
- Explain the purpose of each generated file and folder to build understanding of the project structure.
|
||||
- Demonstrate how to organize code using feature folders or domain-driven design principles.
|
||||
- Show proper separation of concerns with models, services, and data access layers.
|
||||
- Explain the Program.cpp and configuration system in C++ including environment-specific settings.
|
||||
|
||||
## Testing
|
||||
|
||||
- Always include test cases for critical paths of the application.
|
||||
- Guide users through creating unit tests.
|
||||
- Do not emit "Act", "Arrange" or "Assert" comments.
|
||||
- Copy existing style in nearby files for test method names and capitalization.
|
||||
- Explain integration testing approaches for API endpoints.
|
||||
- Demonstrate how to mock dependencies for effective testing.
|
||||
- Show how to test authentication and authorization logic.
|
||||
- Explain test-driven development principles as applied to API development.
|
||||
|
||||
## Performance Optimization
|
||||
|
||||
- Guide users on implementing caching strategies (in-memory, distributed, response caching).
|
||||
- Explain asynchronous programming patterns and why they matter for API performance.
|
||||
- Demonstrate pagination, filtering, and sorting for large data sets.
|
||||
- Show how to implement compression and other performance optimizations.
|
||||
- Explain how to measure and benchmark API performance.
|
||||
|
||||
21
.github/instructions/documentation.instructions.md
vendored
Normal file
21
.github/instructions/documentation.instructions.md
vendored
Normal file
@@ -0,0 +1,21 @@
|
||||
---
|
||||
applyTo: "*.md"
|
||||
---
|
||||
# Project documentation writing guidelines
|
||||
|
||||
## General Guidelines
|
||||
- Write clear and concise documentation.
|
||||
- Use consistent terminology and style.
|
||||
- Include code examples where applicable.
|
||||
|
||||
## Grammar
|
||||
* Use present tense verbs (is, open) instead of past tense (was, opened).
|
||||
* Write factual statements and direct commands. Avoid hypotheticals like "could" or "would".
|
||||
* Use active voice where the subject performs the action.
|
||||
* Write in second person (you) to speak directly to readers.
|
||||
|
||||
## Markdown Guidelines
|
||||
- Use headings to organize content.
|
||||
- Use bullet points for lists.
|
||||
- Include links to related resources.
|
||||
- Use code blocks for code snippets.
|
||||
56
.github/instructions/python.instructions.md
vendored
Normal file
56
.github/instructions/python.instructions.md
vendored
Normal file
@@ -0,0 +1,56 @@
|
||||
---
|
||||
description: 'Python coding conventions and guidelines'
|
||||
applyTo: '**/*.py'
|
||||
---
|
||||
|
||||
# Python Coding Conventions
|
||||
|
||||
## Python Instructions
|
||||
|
||||
- Write clear and concise comments for each function.
|
||||
- Ensure functions have descriptive names and include type hints.
|
||||
- Provide docstrings following PEP 257 conventions.
|
||||
- Use the `typing` module for type annotations (e.g., `List[str]`, `Dict[str, int]`).
|
||||
- Break down complex functions into smaller, more manageable functions.
|
||||
|
||||
## General Instructions
|
||||
|
||||
- Always prioritize readability and clarity.
|
||||
- For algorithm-related code, include explanations of the approach used.
|
||||
- Write code with good maintainability practices, including comments on why certain design decisions were made.
|
||||
- Handle edge cases and write clear exception handling.
|
||||
- For libraries or external dependencies, mention their usage and purpose in comments.
|
||||
- Use consistent naming conventions and follow language-specific best practices.
|
||||
- Write concise, efficient, and idiomatic code that is also easily understandable.
|
||||
|
||||
## Code Style and Formatting
|
||||
|
||||
- Follow the **PEP 8** style guide for Python.
|
||||
- Maintain proper indentation (use 4 spaces for each level of indentation).
|
||||
- Ensure lines do not exceed 79 characters.
|
||||
- Place function and class docstrings immediately after the `def` or `class` keyword.
|
||||
- Use blank lines to separate functions, classes, and code blocks where appropriate.
|
||||
|
||||
## Edge Cases and Testing
|
||||
|
||||
- Always include test cases for critical paths of the application.
|
||||
- Account for common edge cases like empty inputs, invalid data types, and large datasets.
|
||||
- Include comments for edge cases and the expected behavior in those cases.
|
||||
- Write unit tests for functions and document them with docstrings explaining the test cases.
|
||||
|
||||
## Example of Proper Documentation
|
||||
|
||||
```python
|
||||
def calculate_area(radius: float) -> float:
|
||||
"""
|
||||
Calculate the area of a circle given the radius.
|
||||
|
||||
Parameters:
|
||||
radius (float): The radius of the circle.
|
||||
|
||||
Returns:
|
||||
float: The area of the circle, calculated as π * radius^2.
|
||||
"""
|
||||
import math
|
||||
return math.pi * radius ** 2
|
||||
```
|
||||
132
.github/instructions/shell.instructions.md
vendored
Normal file
132
.github/instructions/shell.instructions.md
vendored
Normal file
@@ -0,0 +1,132 @@
|
||||
---
|
||||
description: 'Shell scripting best practices and conventions for bash, sh, zsh, and other shells'
|
||||
applyTo: '**/*.sh'
|
||||
---
|
||||
|
||||
# Shell Scripting Guidelines
|
||||
|
||||
Instructions for writing clean, safe, and maintainable shell scripts for bash, sh, zsh, and other shells.
|
||||
|
||||
## General Principles
|
||||
|
||||
- Generate code that is clean, simple, and concise
|
||||
- Ensure scripts are easily readable and understandable
|
||||
- Add comments where helpful for understanding how the script works
|
||||
- Generate concise and simple echo outputs to provide execution status
|
||||
- Avoid unnecessary echo output and excessive logging
|
||||
- Use shellcheck for static analysis when available
|
||||
- Assume scripts are for automation and testing rather than production systems unless specified otherwise
|
||||
- Prefer safe expansions: double-quote variable references (`"$var"`), use `${var}` for clarity, and avoid `eval`
|
||||
- Use modern Bash features (`[[ ]]`, `local`, arrays) when portability requirements allow; fall back to POSIX constructs only when needed
|
||||
- Choose reliable parsers for structured data instead of ad-hoc text processing
|
||||
|
||||
## Error Handling & Safety
|
||||
|
||||
- Always enable `set -euo pipefail` to fail fast on errors, catch unset variables, and surface pipeline failures
|
||||
- Validate all required parameters before execution
|
||||
- Provide clear error messages with context
|
||||
- Use `trap` to clean up temporary resources or handle unexpected exits when the script terminates
|
||||
- Declare immutable values with `readonly` (or `declare -r`) to prevent accidental reassignment
|
||||
- Use `mktemp` to create temporary files or directories safely and ensure they are removed in your cleanup handler
|
||||
|
||||
## Script Structure
|
||||
|
||||
- Start with a clear shebang: `#!/bin/bash` unless specified otherwise
|
||||
- Include a header comment explaining the script's purpose
|
||||
- Define default values for all variables at the top
|
||||
- Use functions for reusable code blocks
|
||||
- Create reusable functions instead of repeating similar blocks of code
|
||||
- Keep the main execution flow clean and readable
|
||||
|
||||
## Working with JSON and YAML
|
||||
|
||||
- Prefer dedicated parsers (`jq` for JSON, `yq` for YAML—or `jq` on JSON converted via `yq`) over ad-hoc text processing with `grep`, `awk`, or shell string splitting
|
||||
- When `jq`/`yq` are unavailable or not appropriate, choose the next most reliable parser available in your environment, and be explicit about how it should be used safely
|
||||
- Validate that required fields exist and handle missing/invalid data paths explicitly (e.g., by checking `jq` exit status or using `// empty`)
|
||||
- Quote jq/yq filters to prevent shell expansion and prefer `--raw-output` when you need plain strings
|
||||
- Treat parser errors as fatal: combine with `set -euo pipefail` or test command success before using results
|
||||
- Document parser dependencies at the top of the script and fail fast with a helpful message if `jq`/`yq` (or alternative tools) are required but not installed
|
||||
|
||||
```bash
|
||||
#!/bin/bash
|
||||
|
||||
# ============================================================================
|
||||
# Script Description Here
|
||||
# ============================================================================
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
cleanup() {
|
||||
# Remove temporary resources or perform other teardown steps as needed
|
||||
if [[ -n "${TEMP_DIR:-}" && -d "$TEMP_DIR" ]]; then
|
||||
rm -rf "$TEMP_DIR"
|
||||
fi
|
||||
}
|
||||
|
||||
trap cleanup EXIT
|
||||
|
||||
# Default values
|
||||
RESOURCE_GROUP=""
|
||||
REQUIRED_PARAM=""
|
||||
OPTIONAL_PARAM="default-value"
|
||||
readonly SCRIPT_NAME="$(basename "$0")"
|
||||
|
||||
TEMP_DIR=""
|
||||
|
||||
# Functions
|
||||
usage() {
|
||||
echo "Usage: $SCRIPT_NAME [OPTIONS]"
|
||||
echo "Options:"
|
||||
echo " -g, --resource-group Resource group (required)"
|
||||
echo " -h, --help Show this help"
|
||||
exit 0
|
||||
}
|
||||
|
||||
validate_requirements() {
|
||||
if [[ -z "$RESOURCE_GROUP" ]]; then
|
||||
echo "Error: Resource group is required"
|
||||
exit 1
|
||||
fi
|
||||
}
|
||||
|
||||
main() {
|
||||
validate_requirements
|
||||
|
||||
TEMP_DIR="$(mktemp -d)"
|
||||
if [[ ! -d "$TEMP_DIR" ]]; then
|
||||
echo "Error: failed to create temporary directory" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "============================================================================"
|
||||
echo "Script Execution Started"
|
||||
echo "============================================================================"
|
||||
|
||||
# Main logic here
|
||||
|
||||
echo "============================================================================"
|
||||
echo "Script Execution Completed"
|
||||
echo "============================================================================"
|
||||
}
|
||||
|
||||
# Parse arguments
|
||||
while [[ $# -gt 0 ]]; do
|
||||
case $1 in
|
||||
-g|--resource-group)
|
||||
RESOURCE_GROUP="$2"
|
||||
shift 2
|
||||
;;
|
||||
-h|--help)
|
||||
usage
|
||||
;;
|
||||
*)
|
||||
echo "Unknown option: $1"
|
||||
exit 1
|
||||
;;
|
||||
esac
|
||||
done
|
||||
|
||||
# Execute main function
|
||||
main "$@"
|
||||
|
||||
```
|
||||
124
.github/skills/git-commit/SKILL.md
vendored
Normal file
124
.github/skills/git-commit/SKILL.md
vendored
Normal file
@@ -0,0 +1,124 @@
|
||||
---
|
||||
name: git-commit
|
||||
description: 'Execute git commit with conventional commit message analysis, intelligent staging, and message generation. Use when user asks to commit changes, create a git commit, or mentions "/commit". Supports: (1) Auto-detecting type and scope from changes, (2) Generating conventional commit messages from diff, (3) Interactive commit with optional type/scope/description overrides, (4) Intelligent file staging for logical grouping'
|
||||
license: MIT
|
||||
allowed-tools: Bash
|
||||
---
|
||||
|
||||
# Git Commit with Conventional Commits
|
||||
|
||||
## Overview
|
||||
|
||||
Create standardized, semantic git commits using the Conventional Commits specification. Analyze the actual diff to determine appropriate type, scope, and message.
|
||||
|
||||
## Conventional Commit Format
|
||||
|
||||
```
|
||||
<type>[optional scope]: <description>
|
||||
|
||||
[optional body]
|
||||
|
||||
[optional footer(s)]
|
||||
```
|
||||
|
||||
## Commit Types
|
||||
|
||||
| Type | Purpose |
|
||||
| ---------- | ------------------------------ |
|
||||
| `feat` | New feature |
|
||||
| `fix` | Bug fix |
|
||||
| `docs` | Documentation only |
|
||||
| `style` | Formatting/style (no logic) |
|
||||
| `refactor` | Code refactor (no feature/fix) |
|
||||
| `perf` | Performance improvement |
|
||||
| `test` | Add/update tests |
|
||||
| `build` | Build system/dependencies |
|
||||
| `ci` | CI/config changes |
|
||||
| `chore` | Maintenance/misc |
|
||||
| `revert` | Revert commit |
|
||||
|
||||
## Breaking Changes
|
||||
|
||||
```
|
||||
# Exclamation mark after type/scope
|
||||
feat!: remove deprecated endpoint
|
||||
|
||||
# BREAKING CHANGE footer
|
||||
feat: allow config to extend other configs
|
||||
|
||||
BREAKING CHANGE: `extends` key behavior changed
|
||||
```
|
||||
|
||||
## Workflow
|
||||
|
||||
### 1. Analyze Diff
|
||||
|
||||
```bash
|
||||
# If files are staged, use staged diff
|
||||
git diff --staged
|
||||
|
||||
# If nothing staged, use working tree diff
|
||||
git diff
|
||||
|
||||
# Also check status
|
||||
git status --porcelain
|
||||
```
|
||||
|
||||
### 2. Stage Files (if needed)
|
||||
|
||||
If nothing is staged or you want to group changes differently:
|
||||
|
||||
```bash
|
||||
# Stage specific files
|
||||
git add path/to/file1 path/to/file2
|
||||
|
||||
# Stage by pattern
|
||||
git add *.test.*
|
||||
git add src/components/*
|
||||
|
||||
# Interactive staging
|
||||
git add -p
|
||||
```
|
||||
|
||||
**Never commit secrets** (.env, credentials.json, private keys).
|
||||
|
||||
### 3. Generate Commit Message
|
||||
|
||||
Analyze the diff to determine:
|
||||
|
||||
- **Type**: What kind of change is this?
|
||||
- **Scope**: What area/module is affected?
|
||||
- **Description**: One-line summary of what changed (present tense, imperative mood, <72 chars)
|
||||
|
||||
### 4. Execute Commit
|
||||
|
||||
```bash
|
||||
# Single line
|
||||
git commit -m "<type>[scope]: <description>"
|
||||
|
||||
# Multi-line with body/footer
|
||||
git commit -m "$(cat <<'EOF'
|
||||
<type>[scope]: <description>
|
||||
|
||||
<optional body>
|
||||
|
||||
<optional footer>
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
## Best Practices
|
||||
|
||||
- One logical change per commit
|
||||
- Present tense: "add" not "added"
|
||||
- Imperative mood: "fix bug" not "fixes bug"
|
||||
- Reference issues: `Closes #123`, `Refs #456`
|
||||
- Keep description under 72 characters
|
||||
|
||||
## Git Safety Protocol
|
||||
|
||||
- NEVER update git config
|
||||
- NEVER run destructive commands (--force, hard reset) without explicit request
|
||||
- NEVER skip hooks (--no-verify) unless user asks
|
||||
- NEVER force push to main/master
|
||||
- If commit fails due to hooks, fix and create NEW commit (don't amend)
|
||||
645
.github/skills/refactor/SKILL.md
vendored
Normal file
645
.github/skills/refactor/SKILL.md
vendored
Normal file
@@ -0,0 +1,645 @@
|
||||
---
|
||||
name: refactor
|
||||
description: 'Surgical code refactoring to improve maintainability without changing behavior. Covers extracting functions, renaming variables, breaking down god functions, improving type safety, eliminating code smells, and applying design patterns. Less drastic than repo-rebuilder; use for gradual improvements.'
|
||||
license: MIT
|
||||
---
|
||||
|
||||
# Refactor
|
||||
|
||||
## Overview
|
||||
|
||||
Improve code structure and readability without changing external behavior. Refactoring is gradual evolution, not revolution. Use this for improving existing code, not rewriting from scratch.
|
||||
|
||||
## When to Use
|
||||
|
||||
Use this skill when:
|
||||
|
||||
- Code is hard to understand or maintain
|
||||
- Functions/classes are too large
|
||||
- Code smells need addressing
|
||||
- Adding features is difficult due to code structure
|
||||
- User asks "clean up this code", "refactor this", "improve this"
|
||||
|
||||
---
|
||||
|
||||
## Refactoring Principles
|
||||
|
||||
### The Golden Rules
|
||||
|
||||
1. **Behavior is preserved** - Refactoring doesn't change what the code does, only how
|
||||
2. **Small steps** - Make tiny changes, test after each
|
||||
3. **Version control is your friend** - Commit before and after each safe state
|
||||
4. **Tests are essential** - Without tests, you're not refactoring, you're editing
|
||||
5. **One thing at a time** - Don't mix refactoring with feature changes
|
||||
|
||||
### When NOT to Refactor
|
||||
|
||||
```
|
||||
- Code that works and won't change again (if it ain't broke...)
|
||||
- Critical production code without tests (add tests first)
|
||||
- When you're under a tight deadline
|
||||
- "Just because" - need a clear purpose
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Common Code Smells & Fixes
|
||||
|
||||
### 1. Long Method/Function
|
||||
|
||||
```diff
|
||||
# BAD: 200-line function that does everything
|
||||
- async function processOrder(orderId) {
|
||||
- // 50 lines: fetch order
|
||||
- // 30 lines: validate order
|
||||
- // 40 lines: calculate pricing
|
||||
- // 30 lines: update inventory
|
||||
- // 20 lines: create shipment
|
||||
- // 30 lines: send notifications
|
||||
- }
|
||||
|
||||
# GOOD: Broken into focused functions
|
||||
+ async function processOrder(orderId) {
|
||||
+ const order = await fetchOrder(orderId);
|
||||
+ validateOrder(order);
|
||||
+ const pricing = calculatePricing(order);
|
||||
+ await updateInventory(order);
|
||||
+ const shipment = await createShipment(order);
|
||||
+ await sendNotifications(order, pricing, shipment);
|
||||
+ return { order, pricing, shipment };
|
||||
+ }
|
||||
```
|
||||
|
||||
### 2. Duplicated Code
|
||||
|
||||
```diff
|
||||
# BAD: Same logic in multiple places
|
||||
- function calculateUserDiscount(user) {
|
||||
- if (user.membership === 'gold') return user.total * 0.2;
|
||||
- if (user.membership === 'silver') return user.total * 0.1;
|
||||
- return 0;
|
||||
- }
|
||||
-
|
||||
- function calculateOrderDiscount(order) {
|
||||
- if (order.user.membership === 'gold') return order.total * 0.2;
|
||||
- if (order.user.membership === 'silver') return order.total * 0.1;
|
||||
- return 0;
|
||||
- }
|
||||
|
||||
# GOOD: Extract common logic
|
||||
+ function getMembershipDiscountRate(membership) {
|
||||
+ const rates = { gold: 0.2, silver: 0.1 };
|
||||
+ return rates[membership] || 0;
|
||||
+ }
|
||||
+
|
||||
+ function calculateUserDiscount(user) {
|
||||
+ return user.total * getMembershipDiscountRate(user.membership);
|
||||
+ }
|
||||
+
|
||||
+ function calculateOrderDiscount(order) {
|
||||
+ return order.total * getMembershipDiscountRate(order.user.membership);
|
||||
+ }
|
||||
```
|
||||
|
||||
### 3. Large Class/Module
|
||||
|
||||
```diff
|
||||
# BAD: God object that knows too much
|
||||
- class UserManager {
|
||||
- createUser() { /* ... */ }
|
||||
- updateUser() { /* ... */ }
|
||||
- deleteUser() { /* ... */ }
|
||||
- sendEmail() { /* ... */ }
|
||||
- generateReport() { /* ... */ }
|
||||
- handlePayment() { /* ... */ }
|
||||
- validateAddress() { /* ... */ }
|
||||
- // 50 more methods...
|
||||
- }
|
||||
|
||||
# GOOD: Single responsibility per class
|
||||
+ class UserService {
|
||||
+ create(data) { /* ... */ }
|
||||
+ update(id, data) { /* ... */ }
|
||||
+ delete(id) { /* ... */ }
|
||||
+ }
|
||||
+
|
||||
+ class EmailService {
|
||||
+ send(to, subject, body) { /* ... */ }
|
||||
+ }
|
||||
+
|
||||
+ class ReportService {
|
||||
+ generate(type, params) { /* ... */ }
|
||||
+ }
|
||||
+
|
||||
+ class PaymentService {
|
||||
+ process(amount, method) { /* ... */ }
|
||||
+ }
|
||||
```
|
||||
|
||||
### 4. Long Parameter List
|
||||
|
||||
```diff
|
||||
# BAD: Too many parameters
|
||||
- function createUser(email, password, name, age, address, city, country, phone) {
|
||||
- /* ... */
|
||||
- }
|
||||
|
||||
# GOOD: Group related parameters
|
||||
+ interface UserData {
|
||||
+ email: string;
|
||||
+ password: string;
|
||||
+ name: string;
|
||||
+ age?: number;
|
||||
+ address?: Address;
|
||||
+ phone?: string;
|
||||
+ }
|
||||
+
|
||||
+ function createUser(data: UserData) {
|
||||
+ /* ... */
|
||||
+ }
|
||||
|
||||
# EVEN BETTER: Use builder pattern for complex construction
|
||||
+ const user = UserBuilder
|
||||
+ .email('test@example.com')
|
||||
+ .password('secure123')
|
||||
+ .name('Test User')
|
||||
+ .address(address)
|
||||
+ .build();
|
||||
```
|
||||
|
||||
### 5. Feature Envy
|
||||
|
||||
```diff
|
||||
# BAD: Method that uses another object's data more than its own
|
||||
- class Order {
|
||||
- calculateDiscount(user) {
|
||||
- if (user.membershipLevel === 'gold') {
|
||||
+ return this.total * 0.2;
|
||||
+ }
|
||||
+ if (user.accountAge > 365) {
|
||||
+ return this.total * 0.1;
|
||||
+ }
|
||||
+ return 0;
|
||||
+ }
|
||||
+ }
|
||||
|
||||
# GOOD: Move logic to the object that owns the data
|
||||
+ class User {
|
||||
+ getDiscountRate(orderTotal) {
|
||||
+ if (this.membershipLevel === 'gold') return 0.2;
|
||||
+ if (this.accountAge > 365) return 0.1;
|
||||
+ return 0;
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
+ class Order {
|
||||
+ calculateDiscount(user) {
|
||||
+ return this.total * user.getDiscountRate(this.total);
|
||||
+ }
|
||||
+ }
|
||||
```
|
||||
|
||||
### 6. Primitive Obsession
|
||||
|
||||
```diff
|
||||
# BAD: Using primitives for domain concepts
|
||||
- function sendEmail(to, subject, body) { /* ... */ }
|
||||
- sendEmail('user@example.com', 'Hello', '...');
|
||||
|
||||
- function createPhone(country, number) {
|
||||
- return `${country}-${number}`;
|
||||
- }
|
||||
|
||||
# GOOD: Use domain types
|
||||
+ class Email {
|
||||
+ private constructor(public readonly value: string) {
|
||||
+ if (!Email.isValid(value)) throw new Error('Invalid email');
|
||||
+ }
|
||||
+ static create(value: string) { return new Email(value); }
|
||||
+ static isValid(email: string) { return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email); }
|
||||
+ }
|
||||
+
|
||||
+ class PhoneNumber {
|
||||
+ constructor(
|
||||
+ public readonly country: string,
|
||||
+ public readonly number: string
|
||||
+ ) {
|
||||
+ if (!PhoneNumber.isValid(country, number)) throw new Error('Invalid phone');
|
||||
+ }
|
||||
+ toString() { return `${this.country}-${this.number}`; }
|
||||
+ static isValid(country: string, number: string) { /* ... */ }
|
||||
+ }
|
||||
+
|
||||
+ // Usage
|
||||
+ const email = Email.create('user@example.com');
|
||||
+ const phone = new PhoneNumber('1', '555-1234');
|
||||
```
|
||||
|
||||
### 7. Magic Numbers/Strings
|
||||
|
||||
```diff
|
||||
# BAD: Unexplained values
|
||||
- if (user.status === 2) { /* ... */ }
|
||||
- const discount = total * 0.15;
|
||||
- setTimeout(callback, 86400000);
|
||||
|
||||
# GOOD: Named constants
|
||||
+ const UserStatus = {
|
||||
+ ACTIVE: 1,
|
||||
+ INACTIVE: 2,
|
||||
+ SUSPENDED: 3
|
||||
+ } as const;
|
||||
+
|
||||
+ const DISCOUNT_RATES = {
|
||||
+ STANDARD: 0.1,
|
||||
+ PREMIUM: 0.15,
|
||||
+ VIP: 0.2
|
||||
+ } as const;
|
||||
+
|
||||
+ const ONE_DAY_MS = 24 * 60 * 60 * 1000;
|
||||
+
|
||||
+ if (user.status === UserStatus.INACTIVE) { /* ... */ }
|
||||
+ const discount = total * DISCOUNT_RATES.PREMIUM;
|
||||
+ setTimeout(callback, ONE_DAY_MS);
|
||||
```
|
||||
|
||||
### 8. Nested Conditionals
|
||||
|
||||
```diff
|
||||
# BAD: Arrow code
|
||||
- function process(order) {
|
||||
- if (order) {
|
||||
- if (order.user) {
|
||||
- if (order.user.isActive) {
|
||||
- if (order.total > 0) {
|
||||
- return processOrder(order);
|
||||
+ } else {
|
||||
+ return { error: 'Invalid total' };
|
||||
+ }
|
||||
+ } else {
|
||||
+ return { error: 'User inactive' };
|
||||
+ }
|
||||
+ } else {
|
||||
+ return { error: 'No user' };
|
||||
+ }
|
||||
+ } else {
|
||||
+ return { error: 'No order' };
|
||||
+ }
|
||||
+ }
|
||||
|
||||
# GOOD: Guard clauses / early returns
|
||||
+ function process(order) {
|
||||
+ if (!order) return { error: 'No order' };
|
||||
+ if (!order.user) return { error: 'No user' };
|
||||
+ if (!order.user.isActive) return { error: 'User inactive' };
|
||||
+ if (order.total <= 0) return { error: 'Invalid total' };
|
||||
+ return processOrder(order);
|
||||
+ }
|
||||
|
||||
# EVEN BETTER: Using Result type
|
||||
+ function process(order): Result<ProcessedOrder, Error> {
|
||||
+ return Result.combine([
|
||||
+ validateOrderExists(order),
|
||||
+ validateUserExists(order),
|
||||
+ validateUserActive(order.user),
|
||||
+ validateOrderTotal(order)
|
||||
+ ]).flatMap(() => processOrder(order));
|
||||
+ }
|
||||
```
|
||||
|
||||
### 9. Dead Code
|
||||
|
||||
```diff
|
||||
# BAD: Unused code lingers
|
||||
- function oldImplementation() { /* ... */ }
|
||||
- const DEPRECATED_VALUE = 5;
|
||||
- import { unusedThing } from './somewhere';
|
||||
- // Commented out code
|
||||
- // function oldCode() { /* ... */ }
|
||||
|
||||
# GOOD: Remove it
|
||||
+ // Delete unused functions, imports, and commented code
|
||||
+ // If you need it again, git history has it
|
||||
```
|
||||
|
||||
### 10. Inappropriate Intimacy
|
||||
|
||||
```diff
|
||||
# BAD: One class reaches deep into another
|
||||
- class OrderProcessor {
|
||||
- process(order) {
|
||||
- order.user.profile.address.street; // Too intimate
|
||||
- order.repository.connection.config; // Breaking encapsulation
|
||||
+ }
|
||||
+ }
|
||||
|
||||
# GOOD: Ask, don't tell
|
||||
+ class OrderProcessor {
|
||||
+ process(order) {
|
||||
+ order.getShippingAddress(); // Order knows how to get it
|
||||
+ order.save(); // Order knows how to save itself
|
||||
+ }
|
||||
+ }
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Extract Method Refactoring
|
||||
|
||||
### Before and After
|
||||
|
||||
```diff
|
||||
# Before: One long function
|
||||
- function printReport(users) {
|
||||
- console.log('USER REPORT');
|
||||
- console.log('============');
|
||||
- console.log('');
|
||||
- console.log(`Total users: ${users.length}`);
|
||||
- console.log('');
|
||||
- console.log('ACTIVE USERS');
|
||||
- console.log('------------');
|
||||
- const active = users.filter(u => u.isActive);
|
||||
- active.forEach(u => {
|
||||
- console.log(`- ${u.name} (${u.email})`);
|
||||
- });
|
||||
- console.log('');
|
||||
- console.log(`Active: ${active.length}`);
|
||||
- console.log('');
|
||||
- console.log('INACTIVE USERS');
|
||||
- console.log('--------------');
|
||||
- const inactive = users.filter(u => !u.isActive);
|
||||
- inactive.forEach(u => {
|
||||
- console.log(`- ${u.name} (${u.email})`);
|
||||
- });
|
||||
- console.log('');
|
||||
- console.log(`Inactive: ${inactive.length}`);
|
||||
- }
|
||||
|
||||
# After: Extracted methods
|
||||
+ function printReport(users) {
|
||||
+ printHeader('USER REPORT');
|
||||
+ console.log(`Total users: ${users.length}\n`);
|
||||
+ printUserSection('ACTIVE USERS', users.filter(u => u.isActive));
|
||||
+ printUserSection('INACTIVE USERS', users.filter(u => !u.isActive));
|
||||
+ }
|
||||
+
|
||||
+ function printHeader(title) {
|
||||
+ const line = '='.repeat(title.length);
|
||||
+ console.log(title);
|
||||
+ console.log(line);
|
||||
+ console.log('');
|
||||
+ }
|
||||
+
|
||||
+ function printUserSection(title, users) {
|
||||
+ console.log(title);
|
||||
+ console.log('-'.repeat(title.length));
|
||||
+ users.forEach(u => console.log(`- ${u.name} (${u.email})`));
|
||||
+ console.log('');
|
||||
+ console.log(`${title.split(' ')[0]}: ${users.length}`);
|
||||
+ console.log('');
|
||||
+ }
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Introducing Type Safety
|
||||
|
||||
### From Untyped to Typed
|
||||
|
||||
```diff
|
||||
# Before: No types
|
||||
- function calculateDiscount(user, total, membership, date) {
|
||||
- if (membership === 'gold' && date.getDay() === 5) {
|
||||
- return total * 0.25;
|
||||
- }
|
||||
- if (membership === 'gold') return total * 0.2;
|
||||
- return total * 0.1;
|
||||
- }
|
||||
|
||||
# After: Full type safety
|
||||
+ type Membership = 'bronze' | 'silver' | 'gold';
|
||||
+
|
||||
+ interface User {
|
||||
+ id: string;
|
||||
+ name: string;
|
||||
+ membership: Membership;
|
||||
+ }
|
||||
+
|
||||
+ interface DiscountResult {
|
||||
+ original: number;
|
||||
+ discount: number;
|
||||
+ final: number;
|
||||
+ rate: number;
|
||||
+ }
|
||||
+
|
||||
+ function calculateDiscount(
|
||||
+ user: User,
|
||||
+ total: number,
|
||||
+ date: Date = new Date()
|
||||
+ ): DiscountResult {
|
||||
+ if (total < 0) throw new Error('Total cannot be negative');
|
||||
+
|
||||
+ let rate = 0.1; // Default bronze
|
||||
+
|
||||
+ if (user.membership === 'gold' && date.getDay() === 5) {
|
||||
+ rate = 0.25; // Friday bonus for gold
|
||||
+ } else if (user.membership === 'gold') {
|
||||
+ rate = 0.2;
|
||||
+ } else if (user.membership === 'silver') {
|
||||
+ rate = 0.15;
|
||||
+ }
|
||||
+
|
||||
+ const discount = total * rate;
|
||||
+
|
||||
+ return {
|
||||
+ original: total,
|
||||
+ discount,
|
||||
+ final: total - discount,
|
||||
+ rate
|
||||
+ };
|
||||
+ }
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Design Patterns for Refactoring
|
||||
|
||||
### Strategy Pattern
|
||||
|
||||
```diff
|
||||
# Before: Conditional logic
|
||||
- function calculateShipping(order, method) {
|
||||
- if (method === 'standard') {
|
||||
- return order.total > 50 ? 0 : 5.99;
|
||||
- } else if (method === 'express') {
|
||||
- return order.total > 100 ? 9.99 : 14.99;
|
||||
+ } else if (method === 'overnight') {
|
||||
+ return 29.99;
|
||||
+ }
|
||||
+ }
|
||||
|
||||
# After: Strategy pattern
|
||||
+ interface ShippingStrategy {
|
||||
+ calculate(order: Order): number;
|
||||
+ }
|
||||
+
|
||||
+ class StandardShipping implements ShippingStrategy {
|
||||
+ calculate(order: Order) {
|
||||
+ return order.total > 50 ? 0 : 5.99;
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
+ class ExpressShipping implements ShippingStrategy {
|
||||
+ calculate(order: Order) {
|
||||
+ return order.total > 100 ? 9.99 : 14.99;
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
+ class OvernightShipping implements ShippingStrategy {
|
||||
+ calculate(order: Order) {
|
||||
+ return 29.99;
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
+ function calculateShipping(order: Order, strategy: ShippingStrategy) {
|
||||
+ return strategy.calculate(order);
|
||||
+ }
|
||||
```
|
||||
|
||||
### Chain of Responsibility
|
||||
|
||||
```diff
|
||||
# Before: Nested validation
|
||||
- function validate(user) {
|
||||
- const errors = [];
|
||||
- if (!user.email) errors.push('Email required');
|
||||
+ else if (!isValidEmail(user.email)) errors.push('Invalid email');
|
||||
+ if (!user.name) errors.push('Name required');
|
||||
+ if (user.age < 18) errors.push('Must be 18+');
|
||||
+ if (user.country === 'blocked') errors.push('Country not supported');
|
||||
+ return errors;
|
||||
+ }
|
||||
|
||||
# After: Chain of responsibility
|
||||
+ abstract class Validator {
|
||||
+ abstract validate(user: User): string | null;
|
||||
+ setNext(validator: Validator): Validator {
|
||||
+ this.next = validator;
|
||||
+ return validator;
|
||||
+ }
|
||||
+ validate(user: User): string | null {
|
||||
+ const error = this.doValidate(user);
|
||||
+ if (error) return error;
|
||||
+ return this.next?.validate(user) ?? null;
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
+ class EmailRequiredValidator extends Validator {
|
||||
+ doValidate(user: User) {
|
||||
+ return !user.email ? 'Email required' : null;
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
+ class EmailFormatValidator extends Validator {
|
||||
+ doValidate(user: User) {
|
||||
+ return user.email && !isValidEmail(user.email) ? 'Invalid email' : null;
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
+ // Build the chain
|
||||
+ const validator = new EmailRequiredValidator()
|
||||
+ .setNext(new EmailFormatValidator())
|
||||
+ .setNext(new NameRequiredValidator())
|
||||
+ .setNext(new AgeValidator())
|
||||
+ .setNext(new CountryValidator());
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Refactoring Steps
|
||||
|
||||
### Safe Refactoring Process
|
||||
|
||||
```
|
||||
1. PREPARE
|
||||
- Ensure tests exist (write them if missing)
|
||||
- Commit current state
|
||||
- Create feature branch
|
||||
|
||||
2. IDENTIFY
|
||||
- Find the code smell to address
|
||||
- Understand what the code does
|
||||
- Plan the refactoring
|
||||
|
||||
3. REFACTOR (small steps)
|
||||
- Make one small change
|
||||
- Run tests
|
||||
- Commit if tests pass
|
||||
- Repeat
|
||||
|
||||
4. VERIFY
|
||||
- All tests pass
|
||||
- Manual testing if needed
|
||||
- Performance unchanged or improved
|
||||
|
||||
5. CLEAN UP
|
||||
- Update comments
|
||||
- Update documentation
|
||||
- Final commit
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Refactoring Checklist
|
||||
|
||||
### Code Quality
|
||||
|
||||
- [ ] Functions are small (< 50 lines)
|
||||
- [ ] Functions do one thing
|
||||
- [ ] No duplicated code
|
||||
- [ ] Descriptive names (variables, functions, classes)
|
||||
- [ ] No magic numbers/strings
|
||||
- [ ] Dead code removed
|
||||
|
||||
### Structure
|
||||
|
||||
- [ ] Related code is together
|
||||
- [ ] Clear module boundaries
|
||||
- [ ] Dependencies flow in one direction
|
||||
- [ ] No circular dependencies
|
||||
|
||||
### Type Safety
|
||||
|
||||
- [ ] Types defined for all public APIs
|
||||
- [ ] No `any` types without justification
|
||||
- [ ] Nullable types explicitly marked
|
||||
|
||||
### Testing
|
||||
|
||||
- [ ] Refactored code is tested
|
||||
- [ ] Tests cover edge cases
|
||||
- [ ] All tests pass
|
||||
|
||||
---
|
||||
|
||||
## Common Refactoring Operations
|
||||
|
||||
| Operation | Description |
|
||||
| --------------------------------------------- | ------------------------------------- |
|
||||
| Extract Method | Turn code fragment into method |
|
||||
| Extract Class | Move behavior to new class |
|
||||
| Extract Interface | Create interface from implementation |
|
||||
| Inline Method | Move method body back to caller |
|
||||
| Inline Class | Move class behavior to caller |
|
||||
| Pull Up Method | Move method to superclass |
|
||||
| Push Down Method | Move method to subclass |
|
||||
| Rename Method/Variable | Improve clarity |
|
||||
| Introduce Parameter Object | Group related parameters |
|
||||
| Replace Conditional with Polymorphism | Use polymorphism instead of switch/if |
|
||||
| Replace Magic Number with Constant | Named constants |
|
||||
| Decompose Conditional | Break complex conditions |
|
||||
| Consolidate Conditional | Combine duplicate conditions |
|
||||
| Replace Nested Conditional with Guard Clauses | Early returns |
|
||||
| Introduce Null Object | Eliminate null checks |
|
||||
| Replace Type Code with Class/Enum | Strong typing |
|
||||
| Replace Inheritance with Delegation | Composition over inheritance |
|
||||
Reference in New Issue
Block a user