-
Notifications
You must be signed in to change notification settings - Fork 46
[jsweep] Clean create_issue.cjs #13428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -188,11 +188,11 @@ function createParentIssueTemplate(groupId, titlePrefix, workflowName, workflowS | |||||
| */ | ||||||
| async function main(config = {}) { | ||||||
| // Extract configuration | ||||||
| const envLabels = config.labels ? (Array.isArray(config.labels) ? config.labels : config.labels.split(",")).map(label => String(label).trim()).filter(label => label) : []; | ||||||
| const envAssignees = config.assignees ? (Array.isArray(config.assignees) ? config.assignees : config.assignees.split(",")).map(assignee => String(assignee).trim()).filter(assignee => assignee) : []; | ||||||
| const titlePrefix = config.title_prefix || ""; | ||||||
| const envLabels = config.labels ? (Array.isArray(config.labels) ? config.labels : config.labels.split(",")).map(label => String(label).trim()).filter(Boolean) : []; | ||||||
| const envAssignees = config.assignees ? (Array.isArray(config.assignees) ? config.assignees : config.assignees.split(",")).map(assignee => String(assignee).trim()).filter(Boolean) : []; | ||||||
| const titlePrefix = config.title_prefix ?? ""; | ||||||
| const expiresHours = config.expires ? parseInt(String(config.expires), 10) : 0; | ||||||
| const maxCount = config.max || 10; | ||||||
| const maxCount = config.max ?? 10; | ||||||
| const allowedRepos = parseAllowedRepos(config.allowed_repos); | ||||||
| const defaultTargetRepo = getDefaultTargetRepo(config); | ||||||
| const groupEnabled = config.group === true || config.group === "true"; | ||||||
|
|
@@ -261,8 +261,6 @@ async function main(config = {}) { | |||||
|
|
||||||
| processedCount++; | ||||||
|
|
||||||
| const createIssueItem = message; | ||||||
|
|
||||||
| // Merge external resolved temp IDs with our local map | ||||||
| if (resolvedTemporaryIds) { | ||||||
| for (const [tempId, resolved] of Object.entries(resolvedTemporaryIds)) { | ||||||
|
|
@@ -273,7 +271,7 @@ async function main(config = {}) { | |||||
| } | ||||||
|
|
||||||
| // Determine target repository for this issue | ||||||
| const itemRepo = createIssueItem.repo ? String(createIssueItem.repo).trim() : defaultTargetRepo; | ||||||
| const itemRepo = message.repo ? String(message.repo).trim() : defaultTargetRepo; | ||||||
|
|
||||||
| // Validate the repository is allowed | ||||||
| const repoValidation = validateRepo(itemRepo, defaultTargetRepo, allowedRepos); | ||||||
|
|
@@ -305,39 +303,38 @@ async function main(config = {}) { | |||||
| } | ||||||
|
|
||||||
| // Get or generate the temporary ID for this issue | ||||||
| const temporaryId = createIssueItem.temporary_id || generateTemporaryId(); | ||||||
| core.info(`Processing create_issue: title=${createIssueItem.title}, bodyLength=${createIssueItem.body?.length || 0}, temporaryId=${temporaryId}, repo=${qualifiedItemRepo}`); | ||||||
| const temporaryId = message.temporary_id ?? generateTemporaryId(); | ||||||
| core.info(`Processing create_issue: title=${message.title}, bodyLength=${message.body?.length ?? 0}, temporaryId=${temporaryId}, repo=${qualifiedItemRepo}`); | ||||||
|
|
||||||
| // Resolve parent: check if it's a temporary ID reference | ||||||
| let effectiveParentIssueNumber; | ||||||
| let effectiveParentRepo = qualifiedItemRepo; // Default to same repo | ||||||
| if (createIssueItem.parent !== undefined) { | ||||||
| if (message.parent !== undefined) { | ||||||
| // Strip # prefix if present to allow flexible temporary ID format | ||||||
| const parentStr = String(createIssueItem.parent).trim(); | ||||||
| const parentStr = String(message.parent).trim(); | ||||||
| const parentWithoutHash = parentStr.startsWith("#") ? parentStr.substring(1) : parentStr; | ||||||
|
|
||||||
| if (isTemporaryId(parentWithoutHash)) { | ||||||
| // It's a temporary ID, look it up in the map | ||||||
| const resolvedParent = temporaryIdMap.get(normalizeTemporaryId(parentWithoutHash)); | ||||||
| if (resolvedParent !== undefined) { | ||||||
| if (resolvedParent) { | ||||||
|
||||||
| if (resolvedParent) { | |
| if (resolvedParent !== undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using nullish coalescing operator (??) instead of logical OR (||) changes the behavior when config.max is 0. With the old code (||), if config.max was 0 (a falsy value), it would default to 10. With the new code (??), if config.max is 0, it will use 0, which would prevent any issues from being created since the check is processedCount >= maxCount (0 >= 0 = true immediately). If 0 is intended to mean "use default" rather than "create no issues", this is a breaking change. Consider whether config.max = 0 should behave as "no limit" or "create no issues", and if the former, revert to using || operator.