docs: ZEUS Contribution + Code Review Guidelines

This commit is contained in:
Evan Kaloudis
2026-02-15 12:34:27 -05:00
parent 3979463aa3
commit 290d1b43ae
3 changed files with 559 additions and 6 deletions
+274
View File
@@ -0,0 +1,274 @@
# ZEUS Code Review Guidelines
## Why Code Review Matters
Code review is essential for maintaining the quality and security of ZEUS. Since ZEUS handles users' Bitcoin and Lightning funds, thorough review helps prevent bugs that could result in financial loss.
Review is also a bottleneck in open source development. By contributing quality reviews, you provide immediate value to the project while learning the codebase.
## For New Contributors: Review First
If you're new to ZEUS, consider starting with code review before writing code:
### Benefits of Reviewing First
- **Learn the codebase**: Understand patterns, conventions, and architecture
- **Understand quality standards**: See what feedback maintainers provide
- **Provide immediate value**: Help move PRs forward
- **Build relationships**: Get to know maintainers and other contributors
- **Discover contribution opportunities**: Find areas that need improvement
## How to Review a Pull Request
### 1. Understand the Context
Before diving into code:
- Read the PR description and linked issues
- Understand what problem is being solved
- Check if the approach was discussed beforehand
### 2. Check Out and Build
```bash
# Fetch the PR locally
git fetch origin pull/PR_NUMBER/head:pr-PR_NUMBER
git checkout pr-PR_NUMBER
# Install dependencies and build
yarn install
# Run tests
yarn verify
```
### 3. Test the Changes
- **Run the app** on a device or simulator
- **Test the specific feature** being changed
- **Test edge cases**: Empty states, error conditions, large inputs
- **Test related features** that might be affected
- **Test on both platforms** if possible (iOS and Android)
### 4. Review the Code
#### Correctness
- Does the code do what it claims to do?
- Are there logic errors or off-by-one mistakes?
- Are edge cases handled?
- Is error handling appropriate?
#### Security
ZEUS handles Bitcoin funds. Be vigilant about:
- Sensitive data exposure (keys, macaroons, seeds)
- Input validation for payment-related code
- Secure storage usage
- Potential for fund loss scenarios
#### Code Quality
- Is the code readable and maintainable?
- Does it follow existing patterns in the codebase?
- Are TypeScript types used properly (no unnecessary `any`)?
- Is there appropriate test coverage?
#### Architecture
- Does the change fit well with the existing architecture?
- Are there better approaches?
- Will this be easy to maintain long-term?
#### Performance
- Are there unnecessary re-renders?
- Is data fetched efficiently?
- Could this cause performance issues on low-end devices?
#### User Experience
- Is the UI consistent with the rest of the app?
- Does it work well in different themes?
- Is it accessible?
- How does it look with different locales?
- Does it look good at different screen resolutions + system font sizes?
### 5. Review Commits Individually
- Each commit should be logical and self-contained
- Commit messages should be clear and descriptive
- The commit history should tell a story
### 6. Check CI Status
- All automated checks should pass
- If checks fail, the contributor should fix them before review continues
## Providing Feedback
### Be Constructive
- Focus on the code, not the person
- Explain *why* something should change, not just *what*
- Provide suggestions, not just criticism
- Acknowledge good work
### Be Specific
Bad: "This needs improvement"
Good: "This function handles both parsing and validation. Consider splitting it into two functions for better testability."
### Use Appropriate Terminology
- **Nitpick/nit**: Minor suggestion, not blocking (style, naming)
- **Suggestion**: Recommended improvement, use judgment
- **Request changes**: Must be addressed before merge
- **Question**: Seeking clarification, not necessarily a change request
### Example Comments
```markdown
**Nitpick**: Consider renaming `data` to `channelDetails` for clarity.
**Suggestion**: This could be extracted into a custom hook to make it
reusable in other components.
**Security concern**: This logs the invoice which may contain sensitive
payment details. Consider removing or redacting the log.
**Question**: Is there a reason we're not using the existing `formatSats`
utility here?
```
## Review Approval Terminology
Use these terms to communicate your review status:
### ACK (Acknowledged)
Full approval. You have:
- Reviewed the code thoroughly
- Tested the changes
- Approve the PR for merge
Example: `ACK 3a4b5c6 - Tested on iOS and Android, works as expected.`
### tACK (Tested ACK)
You have tested the changes but haven't done a full code review.
Example: `tACK - Tested the new channel open flow on Android, works correctly.`
### cACK (Concept ACK)
You agree with the concept/approach but haven't reviewed the implementation in detail.
Example: `cACK - I agree we need better error handling for failed payments.`
### NACK (Negative ACK)
You recommend against merging. Should include a detailed explanation.
Example: `NACK - This approach could lead to stuck payments in edge cases. See my detailed comment above.`
### Needs Changes
Specific changes are required before you can approve.
## Review Checklist
Use this checklist when reviewing PRs:
### Code Quality
- [ ] Code is readable and well-organized
- [ ] TypeScript types are appropriate (no unnecessary `any`)
- [ ] Follows existing code patterns
- [ ] No dead code or commented-out code
- [ ] Appropriate error handling
### Security
- [ ] No sensitive data logged or exposed
- [ ] Inputs are validated appropriately
- [ ] Secure storage used for sensitive data
- [ ] No potential fund loss scenarios
### Testing
- [ ] Tests pass (`yarn verify`)
- [ ] New functionality has test coverage
- [ ] Manually tested the changes
- [ ] Edge cases tested
### User Experience
- [ ] UI is consistent with existing design
- [ ] Works in different themes (light/dark)
- [ ] Localization strings added for new text
- [ ] Accessible
### Git
- [ ] Commits are atomic and well-described
- [ ] Branch is up-to-date with master
- [ ] No merge commits (rebased)
### Documentation
- [ ] User-facing changes are documented
- [ ] Complex code has appropriate comments
- [ ] PR description explains the changes
## Responding to Reviews
As a PR author, when receiving review feedback:
1. **Thank the reviewer** for their time
2. **Address all comments** - respond or make changes
3. **Use fixup commits** during review for easy re-review
4. **Don't take it personally** - feedback is about the code
5. **Ask for clarification** if feedback is unclear
6. **Squash fixups** before final merge
## Review Etiquette
### For Reviewers
- Review promptly when possible
- Be thorough but don't block on minor issues
- Approve when changes are good enough, not perfect
- Consider the contributor's experience level
- Be encouraging, especially to new contributors
### For Contributors
- Be patient - reviewers are volunteers
- Respond to all feedback
- Keep PRs reasonably sized for easier review
- Don't push changes without addressing feedback
- Be open to suggestions
## Time Expectations
- Simple changes: A few days
- Medium changes: 1-2 weeks
- Large changes: Varies based on complexity
Complex changes may require multiple review rounds. This is normal and helps ensure quality.
## Getting Your PRs Reviewed
To help get faster reviews:
1. **Keep PRs small and focused** - easier to review
2. **Write a clear description** - help reviewers understand context
3. **Include screenshots/videos** for UI changes
4. **Ensure CI passes** before requesting review
5. **Respond promptly** to feedback
6. **Be active in the community** - review others' PRs too
## Questions?
If you have questions about the review process:
- Ask in the [Developer Slack](https://zeusln.slack.com/join/shared_invite/zt-qw205nqa-o4VJJC0zPI7HiSfToZGoVw#/)
- Join the [Telegram](https://t.me/zeusLN)
- Comment on the PR or issue
+283
View File
@@ -0,0 +1,283 @@
# ZEUS Contribution Guidelines
## Introduction
ZEUS is a mobile Bitcoin/Lightning wallet and remote node manager. Like any software that interacts with Bitcoin and the Lightning Network, bugs in ZEUS could potentially result in financial loss for users. As such, we take code quality, security, and proper testing very seriously.
This document outlines the guidelines for contributing to ZEUS. We welcome contributions from the community, but ask that you follow these guidelines to ensure a smooth contribution process.
## Minimum Recommended Skillset
The following skills are recommended for contributors:
- Understanding of Bitcoin and the Lightning Network fundamentals
- Experience with TypeScript and React Native
- Familiarity with mobile development (iOS and/or Android)
- Knowledge of React patterns (state management, component lifecycle, hooks)
- Understanding of asynchronous programming and Promises
- Familiarity with Git and the pull request workflow
- Experience with unit testing (Jest)
## Required Reading
Before contributing, please familiarize yourself with:
- [React Native Documentation](https://reactnative.dev/docs/getting-started)
- [TypeScript Handbook](https://www.typescriptlang.org/docs/handbook/)
- The [Lightning Network specification (BOLTs)](https://github.com/lightning/bolts)
- [LND API Documentation](https://api.lightning.community/) (for LND-related features)
- [Core Lightning Documentation](https://docs.corelightning.org/) (for CLN-related features)
## Development Philosophy
### Share Early, Share Often
We encourage contributors to share their intentions early:
1. **Before starting work**: Check existing issues and pull requests to avoid duplicate efforts
2. **Open an issue first**: For significant changes, open an issue to discuss the approach before writing code
3. **Discuss new dependencies**: Before adding a new library or dependency, discuss it with maintainers first. There may be existing solutions in the codebase or preferred alternatives
4. **Work in progress PRs**: Feel free to open draft PRs early for feedback on your approach
This prevents wasted effort and ensures your contribution aligns with the project's direction.
### Code Quality Standards
- **Readability**: Code is read far more often than it is written. Prioritize clarity over cleverness
- **Maintainability**: Write code that future contributors (including yourself) can understand and modify
- **Security**: Never introduce code that could compromise user funds or privacy
- **Testing**: New features should include appropriate test coverage
## Development Environment Setup
### Prerequisites
- Node.js (LTS version)
- Yarn package manager
- React Native CLI
- Xcode (for iOS development)
- Android Studio (for Android development)
### Getting Started
```bash
# Clone the repository
git clone https://github.com/ZeusLN/zeus.git
cd zeus
# Install dependencies
yarn install
# For iOS
cd ios && pod install && cd ..
# Start the development server
yarn start
# Run on Android
yarn android
# Run on iOS
yarn ios
```
### Running Checks
```bash
# Run all units tests
yarn test
# Type checking
yarn tsc
# Code formatting check
yarn prettier
# Linting
yarn lint
# Run all checks at once (recommended before submitting PR)
yarn verify
```
### Lightning Development Environment
For a quick way to get a Lightning development environment running, check out [Polar](https://github.com/jamaljsr/polar).
### Android Development Notes
When configuring a new node on Android in dev, the `Host` field must be `10.0.2.2` - `127.0.0.1` or `localhost` won't work due to Android emulator networking.
## Code Style Guidelines
### TypeScript/React Native Standards
- Use TypeScript for all new code
- Follow the existing code style in the codebases
- Keep components focused and single-purpose
- Avoid `any` types - use proper typing
### Formatting
- Code is formatted using Prettier
- Run `yarn prettier-write` to format your code
- ESLint is used for linting: `yarn lint`
- Fix all linting issues: `yarn fix-all`
### Naming Conventions
- **Components**: PascalCase (e.g., `PaymentRequest.tsx`)
- **Utilities/helpers**: camelCase (e.g., `formatBalance.ts`)
- **Constants**: UPPER_SNAKE_CASE
- **Stores (MobX)**: PascalCase with `Store` suffix (e.g., `ChannelsStore.ts`)
### File Organization
- Place components in the appropriate directory under `views/` or `components/`
- Keep related files close together
- Stores go in `stores/`
- Utility functions go in `utils/`
- Backend implementations go in `backends/`
## Git Workflow
### Commit Messages
Write clear, descriptive commit messages:
```
component: Short summary of change (50 chars or less)
More detailed explanation if necessary. Wrap lines at 72 characters.
Explain the problem this commit solves and why this approach was chosen.
```
**Commit message prefixes** (use the affected area):
- `views:` for screen/view changes
- `components:` for reusable component changes
- `stores:` for MobX store changes
- `backends:` for backend implementation changes
- `utils:` for utility function changes
- `build:` for build configuration changes
- `docs:` for documentation changes
- `tests:` for test changes
### Atomic Commits
- Each commit should represent a single logical change
- Each commit should ideally pass all verification checks independently (`yarn tsc`, `yarn prettier`, `yarn lint`, `yarn test`)
- Use `git rebase -i` to clean up commit history before requesting review
### Branch Naming
Use descriptive branch names:
- `feature/add-taproot-support`
- `fix/invoice-parsing-error`
- `refactor/channel-list-component`
## Pull Request Process
### Before Submitting
1. Ensure all tests pass: `yarn verify`
2. Test your changes on both iOS and Android if possible
3. Update documentation if you're changing user-facing behavior
4. Rebase on the latest `master` branch
### PR Description
**Important**: Do not delete the pull request template. The template includes a checklist and questions about which platforms you've tested on. This information is essential for reviewers. Please fill out all sections of the template rather than replacing it with a custom description.
The template will ask you to provide:
- Summary of changes
- Which platforms were tested (iOS/Android)
- Checklist of completed items
- Screenshots/videos for UI changes
### Review Process
1. All PRs require at least one approval from a maintainer
2. Address all review feedback
3. Keep discussions focused and constructive
4. Use fixup commits during review, squash before merge
### Merge Requirements
- All CI checks must pass
- At least one maintainer approval
- No unresolved conversations
- Up-to-date with the base branch
## Testing Guidelines
### Unit Tests
- Write tests for new utility functions and business logic
- Use Jest for testing
- Place tests alongside the code they test (e.g., `utils/foo.ts` and `utils/foo.test.ts`)
### Manual Testing
Before submitting a PR, manually test:
1. The specific feature/fix on both platforms when possible
2. Related functionality that might be affected
3. Edge cases and error scenarios
**Important**: If you are making cosmetic/UI changes, please test on both Android and iOS as things don't render exactly the same on both platforms.
### Test Coverage
- Bug fixes should include a test that would have caught the bug
- New features should have reasonable test coverage
- Don't sacrifice test quality for coverage numbers
## Security Considerations
ZEUS handles Bitcoin and Lightning funds. Security is paramount:
- **Never log sensitive data**: Private keys, macaroons, seeds, etc.
- **Validate all inputs**: Especially for payment-related functionality
- **Be cautious with dependencies**: Review new dependencies for security
- **Handle errors gracefully**: Don't expose internal details in error messages
- **Use secure storage**: Sensitive data must use encrypted storage
If you discover a security vulnerability, please report it privately to the maintainers rather than opening a public issue.
## Internationalization (i18n)
ZEUS supports multiple languages:
- All user-facing strings should use the localization system
- Don't hardcode user-facing text in components
- **Only modify `locales/en.json`** when adding new strings - this is the source file for all translations
- Other locale files are managed through [Transifex](https://explore.transifex.com/ZeusLN/zeus/) and should not be modified directly in the repository
- If you'd like to contribute translations, please visit our [Transifex page](https://explore.transifex.com/ZeusLN/zeus/) and request a role for the language you'd like to help with
## Getting Help
- **Issues**: Check existing issues or open a new one
- **Telegram**: [ZEUS Telegram](https://t.me/zeusLN)
- **Slack**: [Developer Slack](https://zeusln.slack.com/join/shared_invite/zt-qw205nqa-o4VJJC0zPI7HiSfToZGoVw#/)
## First-Time Contributors
If you're new to ZEUS:
1. Look for issues labeled `good first issue`
2. Start by reviewing existing PRs to understand the process
3. Consider improving test coverage as a first contribution
4. Don't hesitate to ask questions
We appreciate all contributions, but please avoid trivial changes (typo fixes in comments, whitespace changes) as your first contribution. Focus on substantive improvements.
## Working on Issues
**Note on issue assignment**: GitHub issue assignment is reserved for ZEUS team members. Instead of asking to be assigned an issue, simply leave a comment indicating that you've started working on it. This helps others know the issue is being addressed while keeping the process lightweight.
We prioritize reviewing the first submission for any given issue or feature. However, if a better approach emerges during the review process, we may incorporate ideas from multiple contributions. Don't be discouraged if this happens - it's part of collaborative development and your effort is still valued.
## License
All contributions to ZEUS must be compatible with the AGPLv3 license. By submitting a pull request, you agree that your contribution will be licensed under the project's license.
+2 -6
View File
@@ -130,13 +130,9 @@ ZEUS is proud to be integrated on the following platforms:
## Contributing
Please be sure to run `yarn run tsc` to check for type errors, `yarn run test` to run all tests, and `yarn run prettier` to run the prettier
We welcome contributions from the community. Please read our [Contribution Guidelines](CONTRIBUTING.md).
If you are making cosmetic changes please test on both Android and iOS as things don't render exactly the same on both platforms.
If you're looking for a quick way to get a lightning development environment running, check out [Polar](https://github.com/jamaljsr/polar).
NOTE: When configuring a new node on Android in dev, the `Host` field must be `10.0.2.2` - `127.0.0.1` or `localhost` won't work.
First-time contributors are [highly encouraged to start with code review first](CODE_REVIEW.md), before creating their own Pull Requests.
## Reproducible builds