-
Notifications
You must be signed in to change notification settings - Fork 0
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
temporary force skip in derive #386
Conversation
Wraith CI 👻 Retry RequestCheck the box to re-trigger CI.
|
WalkthroughThe Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
Deploying with Cloudflare Pages
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #386 +/- ##
========================================
- Coverage 7.10% 7.10% -0.01%
========================================
Files 65 65
Lines 2012 2014 +2
Branches 73 73
========================================
Hits 143 143
- Misses 1869 1871 +2 ☔ View full report in Codecov by Sentry. |
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/action/src/ghosts/derive.ts (1 hunks)
Additional comments: 1
packages/action/src/ghosts/derive.ts (1)
- 3-3: The function signature of
derive
has been changed to no longer accept any arguments. Verify that this change does not break any existing calls to this function throughout the codebase.
packages/action/src/ghosts/derive.ts
Outdated
export const derive: Ghost = async () => { | ||
|
||
return 'skipped' |
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.
The derive
function has been modified to return a hardcoded string "skipped". This is a temporary measure and should be clearly documented in the code, including the reason for the change and any conditions for its future removal or reversion.
export const derive: Ghost = async () => {
+ // TEMPORARY: This function now returns 'skipped' to temporarily bypass its usual operations.
+ // This change was made because [REASON].
+ // TODO: Revert this change when [CONDITION].
return 'skipped'
}
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
export const derive: Ghost = async () => { | |
return 'skipped' | |
export const derive: Ghost = async () => { | |
// TEMPORARY: This function now returns 'skipped' to temporarily bypass its usual operations. | |
// This change was made because [REASON]. | |
// TODO: Revert this change when [CONDITION]. | |
return 'skipped' | |
} |
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/action/src/ghosts/derive.ts (1 hunks)
Additional comments: 2
packages/action/src/ghosts/derive.ts (2)
3-4: The
derive
function has been simplified to return a hardcoded string "skipped". Ensure that this change is well-documented and tracked so that the original functionality can be restored when appropriate.3-3: The
derive
function's signature has been modified to no longer accept any arguments. Verify that this change does not affect any other parts of the application that may rely on this function.
// const { owner, repo } = payload | ||
|
||
// const { data: repository } = await octokit.rest.repos.get({ | ||
// owner, | ||
// repo | ||
// }) | ||
|
||
// const { data: list } = await octokit.rest.pulls.list({ | ||
// owner, | ||
// repo, | ||
// state: 'open', | ||
// base: repository.default_branch | ||
// }) | ||
|
||
// if (!list.length) { | ||
// return { | ||
// status: 'skipped', | ||
// detail: 'No open PRs targeting the default branch were found.' | ||
// } | ||
// } | ||
|
||
// const result = list.map((pull) => | ||
// octokit.rest.pulls.updateBranch({ | ||
// owner, | ||
// repo, | ||
// pull_number: pull.number | ||
// }) | ||
// ) | ||
|
||
// await Promise.allSettled(result) | ||
|
||
// return { | ||
// status: 'success', | ||
// detail: `Updated ${result.length} PRs.` | ||
// } |
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.
The original functionality of the derive
function is commented out. It's crucial to track this temporary change with a TODO comment or a ticket in the project's issue tracking system to ensure it is addressed in the future.
+ // TODO: The following code has been temporarily disabled. It should be reviewed and
+ // re-enabled once the conditions for the 'temporary force skip' are no longer applicable.
+ // Relevant ticket: [LINK_TO_TICKET]
// const { owner, repo } = payload
// ...
// return {
// status: 'success',
// detail: `Updated ${result.length} PRs.`
// }
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
// const { owner, repo } = payload | |
// const { data: repository } = await octokit.rest.repos.get({ | |
// owner, | |
// repo | |
// }) | |
// const { data: list } = await octokit.rest.pulls.list({ | |
// owner, | |
// repo, | |
// state: 'open', | |
// base: repository.default_branch | |
// }) | |
// if (!list.length) { | |
// return { | |
// status: 'skipped', | |
// detail: 'No open PRs targeting the default branch were found.' | |
// } | |
// } | |
// const result = list.map((pull) => | |
// octokit.rest.pulls.updateBranch({ | |
// owner, | |
// repo, | |
// pull_number: pull.number | |
// }) | |
// ) | |
// await Promise.allSettled(result) | |
// return { | |
// status: 'success', | |
// detail: `Updated ${result.length} PRs.` | |
// } | |
// TODO: The following code has been temporarily disabled. It should be reviewed and | |
// re-enabled once the conditions for the 'temporary force skip' are no longer applicable. | |
// Relevant ticket: [LINK_TO_TICKET] | |
// const { owner, repo } = payload | |
// const { data: repository } = await octokit.rest.repos.get({ | |
// owner, | |
// repo | |
// }) | |
// const { data: list } = await octokit.rest.pulls.list({ | |
// owner, | |
// repo, | |
// state: 'open', | |
// base: repository.default_branch | |
// }) | |
// if (!list.length) { | |
// return { | |
// status: 'skipped', | |
// detail: 'No open PRs targeting the default branch were found.' | |
// } | |
// } | |
// const result = list.map((pull) => | |
// octokit.rest.pulls.updateBranch({ | |
// owner, | |
// repo, | |
// pull_number: pull.number | |
// }) | |
// ) | |
// await Promise.allSettled(result) | |
// return { | |
// status: 'success', | |
// detail: `Updated ${result.length} PRs.` | |
// } |
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.
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.
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.
Summary by CodeRabbit
derive
function to return "skipped" instead of interacting with external services.