|
1 | 1 | # Copilot Instructions
|
2 | 2 |
|
3 |
| -This document provides instructions for GitHub Copilot to assist with reviewing pull requests and performing tasks based on user prompts. It references existing guidelines and role-specific instructions available in this repository. |
| 3 | +This document provides comprehensive instructions for GitHub Copilot to assist with reviewing pull requests and performing tasks based on user prompts within the Action Destinations repository. These instructions ensure consistent, high-quality reviews and implementations that align with the repository's standards and best practices. |
4 | 4 |
|
5 |
| -## Understanding the repository |
| 5 | +## Primary Responsibilities |
6 | 6 |
|
7 |
| -Read the [README](../README.md) to understand the purpose of this repository and its structure. On high level, this repository contains the following key packages: |
| 7 | +As GitHub Copilot for Action Destinations, you have two primary roles: |
8 | 8 |
|
9 |
| -- [`action-destinations`](../packages/destination-actions): This contains all cloud-mode destinations. All new destinations are registered manually and the destination metadata id is then updated in [index.ts](../packages/destination-actions/src/index.ts). |
10 |
| -- [`browser-destinations`](../packages/browser-destinations): This contains all browser-mode destinations. All new destinations are registered manually and the destination metadata id is then updated in [destination-manifest/index.ts](../packages/destinations-manifest/src/index.ts). |
11 |
| -- [`core`](../packages/core): This contains all the core framework for running the destinations. Any changes made here will affect both cloud-mode and browser-mode destinations. Thorough regression testing is required for these changes. |
12 |
| -- [`destination-subscriptions`](../packages/destination-subscriptions): This package validates event payloads against an action's subscription AST. |
13 |
| -- [`browser-destination-runtime`](../packages/browser-destination-runtime): This package contains the runtime for browser-mode destinations, including the logic for executing actions and handling events. |
| 9 | +1. **Pull Request Reviewer**: Review code changes to ensure they meet quality standards, don't introduce breaking changes, and follow best practices |
| 10 | + |
| 11 | +2. **Implementation Assistant**: Help implement new features, fix bugs, and make improvements to the codebase based on user requirements |
| 12 | + |
| 13 | +## Understanding the Repository Structure |
| 14 | + |
| 15 | +This repository contains the Segment Action Destinations framework, which enables streaming data from Segment to third-party tools. Action Destinations were launched in December 2021 to enable customers with a customizable framework to map Segment event sources to third-party tools. |
| 16 | + |
| 17 | +### Key Packages |
| 18 | + |
| 19 | +- [`destination-actions`](../packages/destination-actions): Cloud-mode destinations that run on Segment's infrastructure. New destinations must be manually registered in [index.ts](../packages/destination-actions/src/index.ts). |
| 20 | +- [`browser-destinations`](../packages/browser-destinations): Browser-mode destinations that run on the client side. New destinations must be manually registered in [destination-manifest/index.ts](../packages/destinations-manifest/index.ts). |
| 21 | +- [`core`](../packages/core): The core framework that powers both cloud and browser destinations. Changes here require thorough regression testing as they affect all destinations. |
| 22 | +- [`destination-subscriptions`](../packages/destination-subscriptions): Validates event payloads against an action's subscription AST. |
| 23 | +- [`browser-destination-runtime`](../packages/browser-destination-runtime): Runtime for browser-mode destinations, handling action execution and event processing. |
| 24 | +- [`cli`](../packages/cli): Command-line tools for interacting with the repository, including scaffolding new destinations and actions. |
| 25 | +- [`ajv-human-errors`](../packages/ajv-human-errors): Wrapper for AJV that produces more user-friendly validation messages. |
| 26 | + |
| 27 | +### Key Concepts |
| 28 | + |
| 29 | +- **Actions**: Functions that define what a destination does with events, including field definitions and perform logic |
| 30 | +- **Destinations**: Collections of actions with shared authentication and configuration |
| 31 | +- **Subscriptions**: JSON configurations that map Segment events to destination actions using FQL queries |
| 32 | +- **Mapping Kit**: Tools for transforming and mapping data between Segment events and destination-specific formats |
| 33 | +- **Batching**: Optional functionality to process multiple events in a single API call for efficiency |
14 | 34 |
|
15 | 35 | ## Reviewing Pull Requests
|
16 | 36 |
|
17 |
| -When reviewing pull requests, Copilot should: |
| 37 | +When reviewing pull requests, thoroughly check the following areas: |
| 38 | + |
| 39 | +### 1. CI Checks and Build Validation |
| 40 | + |
| 41 | +- Verify all CI checks listed in [PR Checks](../docs/pr-guidelines/pr-checks.md) have passed, including: |
| 42 | + - Unit Tests |
| 43 | + - Lint |
| 44 | + - Validate |
| 45 | + - Browser Destination Bundle QA |
| 46 | + - Browser tests: actions-core |
| 47 | + - Required Field Check |
| 48 | + - Test External |
| 49 | + - Code coverage |
| 50 | + |
| 51 | +### 2. Code Quality and Standards |
| 52 | + |
| 53 | +- Follow the [Reviewer Guidelines](../docs/pr-guidelines/pull-request-guidance.md) rigorously |
| 54 | +- Check for appropriate use of framework features like conditionally required fields |
| 55 | +- Verify proper error handling according to [Error Handling Guidelines](../docs/error-handling.md) |
| 56 | +- Look for consistent naming conventions and code style |
| 57 | +- Ensure code is well-documented with clear comments |
| 58 | +- Verify types are properly defined and used |
| 59 | + |
| 60 | +### 3. Breaking Changes Prevention |
| 61 | + |
| 62 | +- Ensure PRs don't introduce breaking changes, especially: |
| 63 | + - Adding new required fields to existing action definitions |
| 64 | + - Changing field types in ways that could break existing integrations |
| 65 | + - Altering the behavior of existing functionality that customers rely on |
| 66 | +- For critical high-volume destinations (e.g., Facebook, Google, Snapchat), recommend using feature flags to safely roll out changes |
| 67 | + - This allows testing in production with limited exposure |
| 68 | + - Helps identify potential issues before affecting all customers |
| 69 | + - Provides a quick rollback mechanism if problems are discovered |
| 70 | + |
| 71 | +### 4. PR Organization |
| 72 | + |
| 73 | +- Recommend splitting changes to multiple destinations into separate PRs |
| 74 | +- Suggest logical commit organization that makes the changes easy to review |
| 75 | +- Check that the PR description clearly explains the changes and testing performed |
| 76 | + |
| 77 | +### 5. Documentation and Grammar |
| 78 | + |
| 79 | +- Verify proper grammar and spelling in all user-facing text |
| 80 | +- Check that field descriptions and labels are clear and helpful |
| 81 | +- Ensure any API changes are properly documented |
| 82 | + |
| 83 | +## Implementing Features and Bug Fixes |
| 84 | + |
| 85 | +When implementing features or fixing bugs based on user prompts, follow these guidelines: |
| 86 | + |
| 87 | +### 1. Code Implementation Best Practices |
| 88 | + |
| 89 | +- Use framework features extensively for consistent behavior: |
| 90 | + - Leverage conditionally required fields for complex validation scenarios |
| 91 | + - Implement proper error handling with appropriate error types from `core/src/errors.ts` |
| 92 | + - Define input fields with accurate types, clear labels, and helpful descriptions |
| 93 | + - Use appropriate format validation for string fields (email, URI, etc.) |
| 94 | +- Implement batching where appropriate using `performBatch` for high-volume destinations |
| 95 | +- Follow authentication best practices as outlined in [authentication.md](../docs/authentication.md) |
| 96 | +- Use the `processHashing` utility for any PII hashing rather than direct crypto calls |
| 97 | +- Utilize mapping kit directives for default field values when appropriate |
| 98 | + |
| 99 | +### 2. Testing Strategy |
| 100 | + |
| 101 | +- Write comprehensive unit tests for all new functionality: |
| 102 | + - Use existing tests as references for style and coverage expectations |
| 103 | + - Mock all external API calls (use `nock` for cloud-mode destinations) |
| 104 | + - Test both success and failure scenarios thoroughly |
| 105 | + - Include tests for edge cases and input validation |
| 106 | + - Test error handling paths to ensure proper error messages |
| 107 | +- Ensure tests are deterministic and don't rely on external services |
| 108 | +- For batching implementations, test various batch sizes and scenarios |
| 109 | + |
| 110 | +### 3. Performance and Security Considerations |
| 111 | + |
| 112 | +- Consider performance implications of changes, especially for high-volume event processing |
| 113 | +- Optimize code for efficiency in action perform methods |
| 114 | +- Never expose or log sensitive information like auth tokens or PII |
| 115 | +- Follow security best practices for handling user data |
| 116 | +- Be mindful of API rate limits when making external requests |
| 117 | +- Use appropriate batch keys for low cardinality to avoid inefficient batching |
| 118 | + |
| 119 | +### 4. Feature-Specific Implementation Guidelines |
| 120 | + |
| 121 | +#### For New Destinations |
| 122 | + |
| 123 | +- Follow the destination creation guide in [docs/create.md](../docs/create.md) |
| 124 | +- Implement proper authentication methods based on destination requirements |
| 125 | +- Add appropriate presets for common use cases to improve user experience |
| 126 | +- Ensure comprehensive error handling with user-friendly, actionable error messages |
| 127 | +- Register new destinations correctly in the appropriate index files |
| 128 | + |
| 129 | +#### For Action Implementations |
| 130 | + |
| 131 | +- Define clear, well-typed input fields with helpful descriptions and examples |
| 132 | +- Implement robust `perform`/`performBatch` methods with proper error handling |
| 133 | +- Use appropriate default values and mapping hints to guide user configuration |
| 134 | +- Consider batching support for high-throughput destinations |
| 135 | +- Implement hooks when appropriate for specialized initialization needs |
| 136 | +- For audience-related functionality, implement appropriate audience support methods |
| 137 | + |
| 138 | +#### For Core Framework Changes |
| 139 | + |
| 140 | +- Be extremely cautious as changes affect all destinations |
| 141 | +- Ensure backward compatibility with existing implementations |
| 142 | +- Add extensive tests covering various scenarios and edge cases |
| 143 | +- Document changes thoroughly and update relevant documentation |
| 144 | +- Consider performance implications across all destination types |
| 145 | + |
| 146 | +## Best Practices for Code Reviews |
18 | 147 |
|
19 |
| -1. Ensure all the required checks mentioned [here](../docs/pr-guidelines/pr-checks.md) have passed. |
20 |
| -2. Follow the expectations outlined in the [Reviewer Guidelines](../docs/pr-guidelines/pull-request-guidance.md). |
21 |
| -3. Suggest splitting changes to multiple destinations into separate pull requests. |
22 |
| -4. Ensure that the pull request doesn't introduce known breaking changes as called out in the [Pull Request Guidance](../docs/pr-guidelines/pull-request-guidance.md). |
| 148 | +### What to Look For |
23 | 149 |
|
24 |
| -## Performing Tasks |
| 150 | +- **Security**: Proper handling of authentication, PII, and sensitive data |
| 151 | +- **Performance**: Efficient code that handles high event volumes well |
| 152 | +- **Maintainability**: Clear code structure, appropriate comments, and documentation |
| 153 | +- **Testing**: Comprehensive test coverage with realistic scenarios |
| 154 | +- **Error Handling**: Proper error handling with clear, actionable messages |
25 | 155 |
|
26 |
| -When performing tasks based on user prompts, Copilot should: |
| 156 | +### Review Process |
27 | 157 |
|
28 |
| -1. Refer to the relevant guidelines and documentation in this repository. |
29 |
| -2. Ensure that any code changes follow the principles outlined in the [Contributor Guidelines](../docs/pr-guidelines/pull-request-guidance.md). |
30 |
| -3. Ensure that code changes are implemented using as many framework features as possible, such as conditionally required fields, error handling etc. |
31 |
| -4. Write clear unit tests for any new functionality or changes made to existing functionality. |
32 |
| - - Use the existing tests as a reference for writing new tests. |
33 |
| - - All external calls should be mocked in the tests. For cloud-mode destinations, we use `nock` to mock external calls. |
| 158 | +1. Start with a high-level understanding of the change purpose |
| 159 | +2. Check CI validation and test results |
| 160 | +3. Review the code changes in detail, focusing on logic and potential issues |
| 161 | +4. Provide specific, actionable feedback on areas for improvement |
| 162 | +5. Suggest alternatives where appropriate, with example code if helpful |
0 commit comments