feat: Update ROADMAP with personal data handling and user moderation features
- Added goals for defining personal data categories and retention obligations. - Included exit criteria for user-level moderation, personal collections, and self-service data export. - Expanded Phase 2 goals to include remote actor moderation and shareable block lists. chore: Update flake.nix to specify main program - Set the main program for the project in the flake configuration. feat: Add issue templates for bug reports, feature requests, and questions - Created structured templates to streamline issue reporting and feature suggestions. docs: Add pull request template for consistent contributions - Introduced a PR template to guide contributors on providing necessary information. docs: Establish a Code of Conduct for community behavior - Implemented a Code of Conduct to promote a respectful and inclusive environment. docs: Create Diversity, Equity, and Inclusion (DEI) statement - Outlined commitment to diversity and inclusion within the FeDIY community. docs: Define Code Review Guidelines for constructive feedback - Established guidelines to ensure respectful and effective code reviews. docs: Implement Security Policy for vulnerability reporting - Created a security policy detailing how to report vulnerabilities and our commitment to addressing them.
This commit is contained in:
@@ -0,0 +1,201 @@
|
||||
# Code Review Guidelines
|
||||
|
||||
Code review is a fundamental part of FeDIY's development process. It's an opportunity to share knowledge, catch issues early, and build a stronger codebase together. This guide outlines how we conduct reviews in a respectful, inclusive, and constructive way.
|
||||
|
||||
## Core Principles
|
||||
|
||||
### 1. **Assume Good Intent**
|
||||
|
||||
Contributors come to FeDIY to build something good. Assume they're doing their best and want their code to be correct.
|
||||
|
||||
### 2. **Focus on the Code, Not the Person**
|
||||
|
||||
Review code and ideas, not the developer. Avoid comments like "This is stupid" or "You should know better." Instead: "This approach could lead to X issue. Could we try Y instead?"
|
||||
|
||||
### 3. **Respect Diverse Perspectives**
|
||||
|
||||
Different developers have different experiences, backgrounds, and approaches. Sometimes there's more than one right way to solve a problem.
|
||||
|
||||
### 4. **Be Constructive**
|
||||
|
||||
Your job as a reviewer isn't to reject ideas but to help the contributor improve their work. Ask questions, suggest alternatives, and explain your reasoning.
|
||||
|
||||
### 5. **Respond Promptly**
|
||||
|
||||
Aim to provide initial feedback within 48 hours. This keeps momentum and shows contributors we value their work.
|
||||
|
||||
### 6. **Be Kind**
|
||||
|
||||
Code review happens asynchronously and in writing. Words can be misinterpreted. Err on the side of kindness and clarity.
|
||||
|
||||
## What to Review
|
||||
|
||||
### Correctness
|
||||
|
||||
- Does the code do what it claims to do?
|
||||
- Are edge cases handled?
|
||||
- Could this code fail in unexpected ways?
|
||||
- Are there potential security, performance, or accessibility issues?
|
||||
|
||||
### Design and Architecture
|
||||
|
||||
- Does this change align with our [ARCHITECTURE.md](ARCHITECTURE.md)?
|
||||
- Does it follow our design patterns?
|
||||
- Could this be implemented more simply?
|
||||
- Will this change make the codebase harder to understand or maintain?
|
||||
|
||||
### Tests
|
||||
|
||||
- Are there tests? Do they cover the happy path and edge cases?
|
||||
- Do tests actually test the claimed behavior?
|
||||
- Would a reviewer understand what the code should do from the tests?
|
||||
|
||||
### Documentation
|
||||
|
||||
- Is the code documented? Is the documentation clear?
|
||||
- Are public APIs documented?
|
||||
- Are complex algorithms explained?
|
||||
- Is there a comment explaining *why* a particular approach was taken?
|
||||
|
||||
### Code Quality
|
||||
|
||||
- Does it follow our style guide?
|
||||
- Is it formatted correctly (`cargo fmt`)?
|
||||
- Does it pass `cargo clippy`?
|
||||
- Are variable and function names clear?
|
||||
|
||||
## How to Review
|
||||
|
||||
### Do
|
||||
|
||||
✅ **Ask questions:** "What does this function return when X happens?"
|
||||
|
||||
✅ **Explain your reasoning:** "I'm concerned about this approach because it could lead to Y. Have you considered Z?"
|
||||
|
||||
✅ **Suggest alternatives:** "Another way to solve this might be..."
|
||||
|
||||
✅ **Praise good work:** "Great catch on this edge case!" or "I like how you handled this—it's elegant."
|
||||
|
||||
✅ **Admit uncertainty:** "I'm not sure about this part. Can you help me understand?"
|
||||
|
||||
✅ **Provide context:** "This pattern conflicts with how we do it in module X. Let's align them."
|
||||
|
||||
✅ **Acknowledge limitations:** "This might be a trade-off between X and Y. Let's discuss."
|
||||
|
||||
### Don't
|
||||
|
||||
❌ **Nitpick style:** Linters and formatters handle this. Focus on substance.
|
||||
|
||||
❌ **Demand perfection:** Good is good enough. Encourage incremental improvement.
|
||||
|
||||
❌ **Approve blindly:** Actually review the code.
|
||||
|
||||
❌ **Leave vague feedback:** "This doesn't seem right" isn't helpful. Be specific.
|
||||
|
||||
❌ **Require your preferred style:** There's often more than one right way.
|
||||
|
||||
❌ **Comment on things outside the PR's scope:** Stay focused.
|
||||
|
||||
❌ **Use harsh language:** Code review isn't a place for snark or condescension.
|
||||
|
||||
## Example Reviews
|
||||
|
||||
### ❌ Not Great
|
||||
|
||||
```
|
||||
This is wrong. Fix it.
|
||||
```
|
||||
|
||||
(Not helpful, unclear, and discouraging.)
|
||||
|
||||
### ✅ Better
|
||||
|
||||
```
|
||||
I'm concerned about this approach. The function reads `self.state` but doesn't
|
||||
check if it's been initialized. In our current code, this could panic.
|
||||
|
||||
Have you considered checking `is_initialized()` first, or would it make sense
|
||||
to initialize in the constructor?
|
||||
|
||||
I'm happy to help think through this if you'd like!
|
||||
```
|
||||
|
||||
(Specific, explains the problem, suggests solutions, and is encouraging.)
|
||||
|
||||
## Review Checklist
|
||||
|
||||
Before approving a PR, confirm:
|
||||
|
||||
- [ ] Code is correct and handles edge cases
|
||||
- [ ] Changes align with project architecture
|
||||
- [ ] Tests are adequate and pass
|
||||
- [ ] Documentation is clear
|
||||
- [ ] Code follows style guidelines (`cargo fmt` and `cargo clippy` pass)
|
||||
- [ ] Commit messages are clear
|
||||
- [ ] No security, performance, or accessibility red flags
|
||||
- [ ] Feedback has been respectful and constructive
|
||||
|
||||
## Handling Disagreement
|
||||
|
||||
Sometimes reviewers and contributors disagree. This is normal and healthy. When this happens:
|
||||
|
||||
1. **State your position clearly.** Explain why you think something should be different.
|
||||
2. **Ask for their perspective.** They might have information you don't.
|
||||
3. **Seek common ground.** Often there's a compromise that works.
|
||||
4. **Escalate if needed.** If you can't agree, involve a maintainer.
|
||||
5. **Respect the decision.** Once a maintainer decides, everyone moves forward.
|
||||
|
||||
Remember: disagreement about code is not personal. We're all trying to build something good.
|
||||
|
||||
## Feedback on Feedback
|
||||
|
||||
Code reviewers are also learning. If you feel feedback was disrespectful or unhelpful:
|
||||
|
||||
1. **Assume good intent.** The reviewer probably wasn't trying to be harsh.
|
||||
2. **Ask for clarification.** "I'm not sure what you mean. Could you elaborate?"
|
||||
3. **Suggest improvement.** "It would help me to understand the concern better if you explained..."
|
||||
4. **Report if needed.** If the reviewer violates our Code of Conduct, report it.
|
||||
|
||||
## Special Cases
|
||||
|
||||
### First-Time Contributors
|
||||
|
||||
Spend a bit more time with PRs from first-time contributors. They're learning the project and may not know all our conventions yet. A bit of extra patience now builds a better long-term contributor.
|
||||
|
||||
### Large Changes
|
||||
|
||||
Big PRs are harder to review. If you're submitting a large change:
|
||||
|
||||
- Open an issue or discussion first to get buy-in on the approach
|
||||
- Consider breaking it into smaller PRs
|
||||
- Explain the overall goal clearly
|
||||
|
||||
As a reviewer of large changes, don't hesitate to ask for clarification or suggest breaking it up.
|
||||
|
||||
### Sensitive Topics
|
||||
|
||||
Some PRs touch on moderation, content policy, security, or inclusion. These need thoughtful, careful review:
|
||||
|
||||
- Take time to understand the context
|
||||
- Consider asking multiple reviewers for input
|
||||
- Be respectful of different perspectives
|
||||
- Remember the human impact of these decisions
|
||||
|
||||
## Resources for Better Reviewing
|
||||
|
||||
- [Google Code Review Guidelines](https://google.github.io/eng-practices/review/)
|
||||
- [Thoughtbot Code Review Etiquette](https://thoughtbot.com/blog/code-review-etiquette)
|
||||
- [Rust API Guidelines](https://rust-lang.github.io/api-guidelines/)
|
||||
- [OWASP Secure Coding Practices](https://owasp.org/www-project-secure-coding-practices-quick-reference-guide/)
|
||||
|
||||
## Questions?
|
||||
|
||||
- Unsure about a review? Ask for help in a comment.
|
||||
- Feedback on the review process? Open an issue tagged `review-process`.
|
||||
- Code of Conduct concerns? See [CODE_OF_CONDUCT.md](CODE_OF_CONDUCT.md).
|
||||
|
||||
---
|
||||
|
||||
Thank you for contributing your time and expertise through code review. It makes FeDIY better!
|
||||
|
||||
**Last Updated:** May 2026
|
||||
Reference in New Issue
Block a user