feat(governance): issue fields v2 follow-up#435
Conversation
|
Warning Review limit reached
More reviews will be available in 3 minutes and 4 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Note
|
🔍 Reviewer Summary for PR #435CI Status: ✅ Recommendations
|
github-actions
Bot
added
area:documentation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the organization issue and project field configurations, introducing version 2 of the schema. It defines custom fields, policies, hidden fields, and iteration settings, while updating the documentation and the validation script to enforce these rules. The code review identified several opportunities to improve robustness and maintainability: moving migration guidelines to a central migration document, adding defensive checks in the validation script to prevent runtime crashes on null or undefined fields, and replacing a fragile array slice with an explicit list of expected types.
| - If a custom field is renamed, update `.github/issue-fields.yml`, this document, and validators in the same PR. | ||
| - Before deleting a custom field, migrate affected data/views first, then update canonical YAML/docs/validators. |
There was a problem hiding this comment.
According to the repository guidelines, migration maps and notes should be documented in a central /docs/MIGRATION.md file to ensure contributors can easily follow migration rules across the repository. Please consider moving these custom field migration and renaming guidelines to /docs/MIGRATION.md or adding a reference link to it.
References
- Document migration maps and notes in a central
/docs/MIGRATION.mdfile to ensure contributors can follow migration rules mentioned in README files across the repository.
| 'Technical Impact', | ||
| 'Spec Link', | ||
| ]; | ||
| const keySet = new Set(customFields.map((f) => f.key)); |
There was a problem hiding this comment.
To prevent potential runtime TypeError crashes if any element in customFields is null or undefined, use optional chaining (f?.key) and filter out any falsy values.
const keySet = new Set(customFields.map((f) => f?.key).filter(Boolean));
| for (const field of customFields) { | ||
| if (!field.key || typeof field.key !== 'string') fail('Each custom field requires a string key'); | ||
| if (!field.type || !validTypes.has(field.type)) { | ||
| fail(`Custom field "${field.key || 'unknown'}" has invalid type "${field.type}"`); | ||
| } |
There was a problem hiding this comment.
If field is null or undefined, accessing field.key or field.type will throw a TypeError and crash the validation script. Add a safety check to ensure field is a valid object before accessing its properties, and use continue to skip invalid fields safely.
for (const field of customFields) {
if (!field || !field.key || typeof field.key !== 'string') {
fail('Each custom field requires a string key');
continue;
}
if (!field.type || !validTypes.has(field.type)) {
fail('Custom field "' + field.key + '" has invalid type "' + field.type + '"');
}
| prDefaults.type_label, | ||
| ...collectKeys(fieldMappings.Status), | ||
| ...collectKeys(fieldMappings.Priority), | ||
| ...collectKeys(fieldMappings.Type).slice(0, 5), |
There was a problem hiding this comment.
Slicing the first 5 keys of fieldMappings.Type is fragile because any reordering or addition of keys in .github/issue-fields.yml could break the validation. Since type:task and type:chore are already checked via the defaults, you can explicitly list the remaining expected types (type:bug, type:feature, type:documentation) to make this check robust and self-documenting.
'type:bug',
'type:feature',
'type:documentation',
|
Superseded note for history: this branch line is now considered superseded by the rollout-v2 line ( |
1 participant
name: "Pull Request"
about: "General changes, refactors, and maintenance"
title: "PR: {short summary}"
labels: ["status:needs-review"]
General Pull Request
Linked issues
Closes #
Changelog
Added
Changed
Fixed
Removed
Risk Assessment
Risk Level:
Potential Impact:
Mitigation Steps:
How to Test
Prerequisites
Test Steps
Expected Results
Edge Cases to Verify
Checklist (Global DoD / PR)
last_updatedandversion)References