Revert /lgtm command #5718
Merged
Revert /lgtm command #5718
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AaronCrawfis
approved these changes
Jun 14, 2023
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
youngbupark
commented
Jun 14, 2023
| @@ -26,7 +30,7 @@ async function handleIssueCommentCreate({ github, context }) { | |||
| const issue = context.issue; | |||
| const isFromPulls = !!payload.issue.pull_request; | |||
| const commentBody = payload.comment.body; | |||
| const username = context.actor.toLowerCase(); | |||
| const username = context.actor; | |||
There was a problem hiding this comment.
lowercasing doesn't make sense.
youngbupark
commented
Jun 14, 2023
| try { | ||
| await handleIssueCommentCreate({ github, context }); | ||
| } catch (error) { | ||
| console.log(`[handleIssueCommentCreate] unexpected error: ${error}`); |
There was a problem hiding this comment.
added try-catch here to catch unhandled exceptions.
Test Results2 699 tests ±0 2 692 ✔️ ±0 1m 58s ⏱️ +3s Results for commit 1cc4acf. ± Comparison against base commit b8b22c7. This pull request removes 2 and adds 2 tests. Note that renamed tests count towards both. |
|
For the detailed report, please go to
|
nithyatsu
pushed a commit
that referenced
this pull request
Jun 21, 2023
# Description /lgtm will not work since it always uses bot username as a approval. ## Issue reference <!-- We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation. --> Fixes: #issue_number ## Checklist Please make sure you've completed the relevant tasks for this PR, out of the following list: * [ ] Code compiles correctly * [ ] Adds necessary unit tests for change * [ ] Adds necessary E2E tests for change * [ ] Unit tests passing * [ ] Extended the documentation / Created issue for it ## Auto-generated summary <!-- GitHub Copilot for docs will auto-generate a summary of the PR --> <!-- copilot:all --> ### <samp>🤖 Generated by Copilot at 9abec5f</samp> ### Summary 🗑️🛠️🚫 <!-- 1. 🗑️ - This emoji represents the removal or deletion of something, which in this case is the `/lgtm` command and function. It conveys the idea of simplifying or cleaning up the script by getting rid of unused or unnecessary code. 2. 🛠️ - This emoji represents the fixing or improvement of something, which in this case is the error handling for the issue comment handler. It conveys the idea of enhancing the robustness or reliability of the script by handling possible errors or exceptions that might occur when processing comments. 3. 🚫 - This emoji represents the prohibition or prevention of something, which in this case is the conflict with the GitHub code review feature. It conveys the idea of avoiding or resolving a potential problem or issue that might arise from having two different ways of approving pull requests (the `/lgtm` command and the GitHub code review feature). --> This pull request simplifies the `radius-bot.js` script by removing the `/lgtm` command and adding error handling. This avoids conflicts with the GitHub code review feature and makes the script more robust. > _Sing, O Muse, of the valiant coder who dared_ > _To alter the script of the radiant radius-bot,_ > _Removing the `/lgtm` command, which once declared_ > _The approval of changes with a swift and simple dot._ ### Walkthrough * Catch and log errors in issue comment handling (`[link](https://github.com/project-radius/radius/pull/5718/files?diff=unified&w=0#diff-b94c0e124b5b65fb6fa8785928c5331547c124a96ecc225f93df9445a29a5109L19-R23)`)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
/lgtm will not work since it always uses bot username as a approval.
Issue reference
Fixes: #issue_number
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list:
Auto-generated summary
🤖 Generated by Copilot at 9abec5f
Summary
🗑️🛠️🚫
This pull request simplifies the
radius-bot.jsscript by removing the/lgtmcommand and adding error handling. This avoids conflicts with the GitHub code review feature and makes the script more robust.Walkthrough
[link](https://github.com/project-radius/radius/pull/5718/files?diff=unified&w=0#diff-b94c0e124b5b65fb6fa8785928c5331547c124a96ecc225f93df9445a29a5109L19-R23))