hackathon/docs/quality/PULL_REQUEST_TEMPLATE.md

# Pull Request Template
**Project**: HTTP Sender Plugin (HSP)

---

## 📝 PR Title
<!-- Use format: [TYPE] Brief description -->
<!-- Examples:
     feat: implement BufferManager with FIFO overflow
     fix: correct retry logic in HttpPollingAdapter
     refactor: improve DataCollectionService naming
     test: add integration tests for gRPC transmission
     docs: update architecture diagrams
-->

---

## 📋 Summary
<!-- Provide a brief description of what this PR does (2-3 sentences) -->




---

## 🎯 Related Requirements
<!-- List all requirements this PR implements/addresses -->
<!-- Use format: - Req-XX: Description -->
<!-- Find requirements in: docs/requirements/DataCollector SRS.md -->

- Req-FR-XX: [Requirement description]
- Req-NFR-XX: [Requirement description]
- Req-Arch-XX: [Requirement description]

---

## ✅ TDD Compliance ⚠️ MANDATORY

### Git History Verification
<!-- Reviewers will verify the following -->

- [ ] **Tests committed BEFORE implementation** (check Git timestamps)
- [ ] **Git history shows RED-GREEN-REFACTOR cycle** (test → feat → refactor commits)
- [ ] **Commit messages follow TDD pattern** (`test: ... (RED)`, `feat: ... (GREEN)`, `refactor: ...`)

### Test Quality
- [ ] **Tests follow AAA pattern** (Arrange-Act-Assert)
- [ ] **Test names describe behavior**: `shouldDoX_whenY()`
- [ ] **Edge cases covered** (boundary conditions, nulls, empty)
- [ ] **Error scenarios tested** (exceptions, timeouts, failures)
- [ ] **Both happy path and sad path tested**

### Coverage Verification
- [ ] **Line coverage**: ___% (target: ≥95%)
- [ ] **Branch coverage**: ___% (target: ≥90%)
- [ ] **JaCoCo report generated**: `mvn jacoco:report` (attach link or screenshot)
- [ ] **CI coverage check passes**

### TDD Evidence
<!-- Provide Git log excerpt showing RED-GREEN-REFACTOR cycle -->
```
Example:
abc1234 test: add BufferManager offer() test (RED)
def5678 feat: implement BufferManager offer() method (GREEN)
ghi9012 refactor: add javadoc to BufferManager

jkl3456 test: add BufferManager overflow test (RED)
mno7890 feat: implement FIFO overflow handling (GREEN)
pqr1234 refactor: improve naming in overflow logic
```

**Git Log**:
```
<!-- Paste relevant commit history here -->


```

---

## 🏗️ Architecture Impact

### Hexagonal Architecture Compliance
- [ ] **Follows hexagonal architecture** (domain → ports → adapters)
- [ ] **Port interfaces not violated** (no direct adapter access from core)
- [ ] **Domain models remain pure** (no infrastructure dependencies)
- [ ] **Adapters properly isolated** (implement port interfaces)
- [ ] **Dependency direction correct**: Core → Ports ← Adapters

### Components Affected
<!-- Check all that apply -->
- [ ] Domain Models (`com.siemens.coreshield.hsp.domain`)
- [ ] Application Services (`com.siemens.coreshield.hsp.application`)
- [ ] Port Interfaces (`com.siemens.coreshield.hsp.ports.inbound|outbound`)
- [ ] Inbound Adapters (`com.siemens.coreshield.hsp.adapter.inbound`)
- [ ] Outbound Adapters (`com.siemens.coreshield.hsp.adapter.outbound`)
- [ ] Configuration
- [ ] Build/Deployment

### Architecture Diagram Impact
- [ ] **No architecture changes** (implementation only)
- [ ] **Minor architecture changes** (new adapter, new port method)
- [ ] **Major architecture changes** (new component, structural change)

<!-- If major changes, update diagrams in docs/diagrams/ -->

---

## 🧪 Testing

### Unit Tests
- [ ] **Unit tests added** for all new code
- [ ] **Unit tests updated** for changed code
- [ ] **All unit tests pass** locally: `mvn test`
- [ ] **Mock objects used appropriately** (external dependencies mocked)

### Integration Tests
<!-- Check if applicable -->
- [ ] **Integration tests added** (if new component or adapter)
- [ ] **Integration tests updated** (if component behavior changed)
- [ ] **All integration tests pass** locally: `mvn verify -P integration-tests`
- [ ] **External systems mocked** (WireMock for HTTP, gRPC test server)

### Performance Tests
<!-- Check if applicable to performance-sensitive code -->
- [ ] **Performance tests added** (if performance-sensitive code)
- [ ] **Benchmarks run** (if applicable)
- [ ] **No performance regression** (compared to baseline)

### Manual Testing
- [ ] **Manual testing performed** (describe below)

**Manual Testing Details**:
```
<!-- Describe what you tested manually -->
Example:
- Tested BufferManager with 1000 concurrent threads
- Verified overflow behavior discards oldest items
- Confirmed statistics are accurate under load
```

---

## 🔒 Thread Safety (if applicable)

<!-- Only fill out if code involves concurrency/shared state -->

- [ ] **Thread-safe implementation** (if required by requirements)
- [ ] **Immutability used** (value objects, final fields)
- [ ] **Proper synchronization** (`synchronized`, `Lock`, concurrent collections)
- [ ] **Atomic operations used** (`AtomicLong`, `AtomicInteger` for counters)
- [ ] **No race conditions** (shared state properly protected)
- [ ] **Stress tests exist** (concurrent tests with 100+ threads)

**Thread Safety Justification**:
```
<!-- Explain why this code is thread-safe, or why it doesn't need to be -->


```

---

## 🔐 Security Considerations

### Input Validation
- [ ] **All external inputs validated** (HTTP responses, configuration)
- [ ] **Size limits enforced** (Req-FR-21: 1MB limit if applicable)
- [ ] **URL validation** (protocol, host, port if applicable)
- [ ] **JSON validation** (schema compliance if applicable)

### Data Handling
- [ ] **Sensitive data not logged** (no raw data in logs)
- [ ] **Base64 encoding used** (for binary data if applicable)
- [ ] **No credentials in code** (use configuration)

### Security Review Needed?
- [ ] **No security-sensitive code** (skip security review)
- [ ] **Security review recommended** (involves authentication, data handling, external input)

---

## 📚 Documentation

### Code Documentation
- [ ] **Javadoc added** for all public APIs (classes, methods, interfaces)
- [ ] **Requirement traceability in Javadoc**: `@requirement Req-FR-XX`
- [ ] **Complex logic explained** (comments or Javadoc)
- [ ] **TODOs tracked** (with ticket numbers if applicable)

### External Documentation
- [ ] **README updated** (if public APIs changed)
- [ ] **Architecture diagrams updated** (if structure changed)
- [ ] **Configuration examples updated** (if config changed)
- [ ] **Operations documentation updated** (if deployment changed)

**Documentation Changes**:
<!-- List files changed or reference "None" -->
-
-
-

---

## ⚙️ Configuration Changes

<!-- Only fill out if configuration changed -->

- [ ] **No configuration changes**
- [ ] **Configuration schema updated** (hsp-config.json)
- [ ] **Configuration validation updated** (ConfigurationValidator)
- [ ] **Configuration example updated** (docs/examples/)
- [ ] **Backward compatible** (or migration path documented)

**Configuration Impact**:
```
<!-- Describe configuration changes -->


```

---

## 🚀 Build and Deployment

### Build Verification
- [ ] **Maven build succeeds**: `mvn clean package` ✅
- [ ] **All tests pass**: `mvn test` ✅
- [ ] **Coverage check passes**: `mvn jacoco:check` ✅
- [ ] **No build warnings** (deprecations, unchecked casts)

### CI/CD Pipeline
- [ ] **CI pipeline passing** (all checks green)
- [ ] **No new dependencies** (or dependencies documented below)
- [ ] **Fat JAR builds correctly** (if applicable)

**New Dependencies** (if any):
<!-- List new Maven dependencies -->
```xml
<!-- Example:
<dependency>
    <groupId>com.google.guava</groupId>
    <artifactId>guava</artifactId>
    <version>32.1.3-jre</version>
</dependency>
-->


```

---

## 🔄 Migration/Upgrade Path

<!-- Only fill out if breaking changes -->

- [ ] **No breaking changes**
- [ ] **Breaking changes documented** (describe below)
- [ ] **Migration guide provided** (for users/operators)

**Breaking Changes**:
```
<!-- Describe breaking changes and migration steps -->


```

---

## 📸 Screenshots/Logs (if applicable)

<!-- Add screenshots for UI changes, logs for behavior changes -->

**Coverage Report**:
<!-- Attach screenshot or link to JaCoCo HTML report -->


**Test Results**:
<!-- Attach screenshot or paste test output if relevant -->


**Logs/Output**:
<!-- Paste relevant log output if demonstrating behavior -->
```


```

---

## ✅ Pre-Merge Checklist

<!-- Verify before requesting review -->

### Code Quality
- [ ] **Self-reviewed code** (read through all changes)
- [ ] **No commented-out code** (removed debug code)
- [ ] **No debug statements** (removed `System.out.println`, excessive logging)
- [ ] **Code formatted consistently** (IDE auto-format applied)

### Testing
- [ ] **All tests passing locally** (`mvn test`)
- [ ] **Coverage thresholds met** (95%/90%)
- [ ] **Integration tests passing** (if applicable)

### Documentation
- [ ] **Javadoc complete** (all public APIs)
- [ ] **README updated** (if needed)
- [ ] **Requirement traceability added** (in Javadoc)

### Process
- [ ] **Feature branch up-to-date** with develop (rebased or merged)
- [ ] **Conflicts resolved** (if any)
- [ ] **CI pipeline green** (all checks passing)

---

## 👥 Reviewers

### Required Reviewers
<!-- Tag specific reviewers if needed -->

- [ ] **Senior Developer**: @[username] (for architectural changes)
- [ ] **QA Engineer**: @[username] (for test changes)
- [ ] **Security Specialist**: @[username] (for security-sensitive code)

### Review Priority
<!-- Check one -->
- [ ] **Low Priority**: Minor change, no rush
- [ ] **Normal Priority**: Standard feature/fix
- [ ] **High Priority**: Blocking other work
- [ ] **Critical Priority**: Production issue, needs immediate review

---

## 💬 Additional Notes

<!-- Any additional context, explanations, or questions for reviewers -->




---

## 🔗 Related Issues/PRs

<!-- Link to related Gitea issues or PRs -->

- Closes #XX (issue number)
- Related to #YY
- Depends on #ZZ

---

## ✍️ Author Checklist

<!-- Final verification before submitting -->

- [ ] I have read the [Code Review Guidelines](CODE_REVIEW_GUIDELINES.md)
- [ ] I have read the [TDD Compliance Checklist](TDD_COMPLIANCE_CHECKLIST.md)
- [ ] I have followed TDD methodology (tests before code)
- [ ] I have self-reviewed my code
- [ ] I have tested all changes locally
- [ ] I have updated documentation
- [ ] I have added requirement traceability
- [ ] I am ready for code review

---

**Submitted by**: @[your-username]
**Date**: [YYYY-MM-DD]
**Branch**: `feature/[branch-name]` → `develop` (or `main`)

---

<!--
REVIEWER INSTRUCTIONS:
1. Review using the Code Review Guidelines checklist
2. Verify TDD compliance (Git history, tests before code)
3. Check hexagonal architecture boundaries
4. Verify thread safety (if applicable)
5. Check test coverage (≥95%/90%)
6. Provide constructive feedback with categories (🚨 BLOCKER, ⚠️ MAJOR, 💡 SUGGESTION)
7. Approve when all blockers resolved and checklist complete
-->