fix: improvements on follow-up #36
Conversation
Replaced Context with ContextPlugin across multiple files for consistency.
Unused files (1)
Unused dependencies (1)
|
Updated test cases to use the run function instead of runPlugin.
Updated default log level to use LOG_LEVEL.INFO constant instead of a string.
Deleted the generated CommonJS distribution file.
Deleted the generated CommonJS distribution file.
Removed the yarn.lock file to reset dependencies.
Removed the outdated express_plugin mock file from tests.
Removed the outdated express_plugin mock file from tests.
Upgraded @octokit/rest and @types/node, and removed unused dependencies.
Added a new input field 'signature' to capture kernel payload signature.
Included kernel public key in environment variables and workflows.
Included kernel public key in environment variables and workflows.
This change eliminates the Node.js setup step from the compute.yml GitHub Actions workflow.
Deleted payload parsing and validation logic along with associated dependencies.
Updated logic to unassign users and send reminders, corrected activity date handling.
Added logic to collect and analyze linked pull request reminders and optimized log management.
Removed redundant deadline management in task-update and fixed function name in watch-user-activity.
Refactored task update logic for better readability and efficiency.
Introduce pullRequestRequired flag in configuration and improve comment retrieval logic.
Deleted task-deadline.ts and updated package.json dependencies.
Added ESM support in jest, adjusted test scripts and new config options.
Added actorId to createEvent function, adjusted log levels.
Replaces error throw with logger error and graceful return.
Replaces error throw with logger error and graceful return.
Replaces error throw with logger error and graceful return.
Co-authored-by: アレクサンダー.eth <[email protected]>
|
|
||
| return true; | ||
| return { message: "OK" }; |
There was a problem hiding this comment.
How about a statusCode: 200 instead
There was a problem hiding this comment.
Sure, will change it. Thought it was not useful because the network call already sends a 200, this is the body.
| await remindAssigneesForIssue(context, issue); | ||
| const activityEvent = (await getAssigneesActivityForIssue(context, issue, handledMetadata.taskAssignees)) | ||
| .filter((o) => eventWhitelist.includes(o.event as TimelineEvent)) | ||
| .shift(); |
There was a problem hiding this comment.
This looks like this can shift an undefined value, I don't think this is type safe. Perhaps you should check if its undefined and throw a specific error for that if you must typecast TimelineEvent.
There was a problem hiding this comment.
undefined is checked, the shift is on purpose, to check if there was any activity, and if not define the last activity as when the user got assigned. The one that cannot be undefined is assignedEvent because if it is it means the user never got assigned, and yes in this case I exit the issue check.
| .shift(); | ||
|
|
||
| const assignedDate = DateTime.fromISO(assignedEvent.created_at); | ||
| const activityDate = activityEvent?.created_at ? DateTime.fromISO(activityEvent.created_at) : undefined; |
There was a problem hiding this comment.
I think it should never be undefined. You should throw for that.
There was a problem hiding this comment.
activityDate can be undefined if the user just assigned itself and never did anything afterwards (which is a common scenario in our repos). When this happens the last activity points to when the user was assigned (coincides with the user sending a /start command).
Enhanced test mocks to better handle event creation and comments.
Enhanced test mocks to better handle event creation and comments.
Enhanced test mocks to better handle event creation and comments.
gentlementlegen
merged commit 398f17e
into
ubiquity-os-marketplace:development
|
I re-enabled the workflow as well. |
Resolves #23
QA: Meniole#2