-
Notifications
You must be signed in to change notification settings - Fork 51
[PROD RELEASE] - AI Workflows #1709
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
Conversation
fix(PM-2372): remove the phase name which is already added
fix(PM-2917, PM-2927): remove unneccessary prop for ai reviewer
| filters: &filters-dev | ||
| branches: | ||
| only: ["develop"] | ||
| only: ["develop", "pm-2917"] |
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.
[maintainability]
Adding the branch pm-2917 to the develop branch filter may be intentional for testing purposes, but ensure this is not left in production configurations if it's meant for temporary use. Consider documenting the purpose of this branch inclusion elsewhere if it's part of a longer-term strategy.
|
|
||
| // If current reviewer is a member review, allow selecting phases even if already assigned to others. | ||
| // Only exclude assigned phases for ai reviewers. | ||
| if (!!reviewer.isMemberReview && assignedPhaseIds.has(phase.phaseId || phase.id) && !isCurrentlySelected) { |
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.
[💡 style]
The condition !!reviewer.isMemberReview is redundant because isMemberReview is already a boolean. Consider simplifying it to reviewer.isMemberReview.
|
|
||
| // If current reviewer is a member review, allow selecting phases even if already assigned to others. | ||
| // Only exclude assigned phases for ai reviewers. | ||
| if (!!reviewer.isMemberReview && assignedPhaseIds.has(phase.phaseId || phase.id) && !isCurrentlySelected) { |
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.
[correctness]
The logic here seems to be excluding phases already assigned to other reviewers only for AI reviewers. This could lead to unexpected behavior if a member reviewer is mistakenly flagged as an AI reviewer. Ensure that isMemberReview is correctly set for all reviewers.
|
|
||
| if (challenge.reviewers && Array.isArray(challenge.reviewers)) { | ||
| challenge.reviewers = challenge.reviewers.map(reviewer => { | ||
| if (reviewer.isMemberReview === false) { |
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.
[💡 style]
The condition reviewer.isMemberReview === false is explicitly checking for false. Consider using a more concise check like !reviewer.isMemberReview unless you specifically need to distinguish between false and undefined.
| console.error('An unexpected error occurred while getting auth token') | ||
| } | ||
| const errorMessage = error && error.message ? error.message : error | ||
| console.error('An unexpected error occurred while getting auth token', errorMessage) |
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.
[maintainability]
Consider logging the entire error object instead of just the message. This can provide more context for debugging, such as stack traces or additional properties that might be present on the error object.
https://topcoder.atlassian.net/browse/PM-3078