Skip to content
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

On delete mode new comment always will be created #361

Open
ViacheslavKudinov opened this issue Mar 16, 2024 · 3 comments
Open

On delete mode new comment always will be created #361

ViacheslavKudinov opened this issue Mar 16, 2024 · 3 comments

Comments

@ViacheslavKudinov
Copy link

ViacheslavKudinov commented Mar 16, 2024

Hello,
i have the situation when i change mode dynamically based on the result of another step, but on delete mode extra comment will be temporarily added all the time due to current logic.

I use this Action to add comment on PR in case if TFLinter finds some concerns based on exit code.
When TFLinter finds concern, the Action adds comment as it runs based on

      - name: 'Set comment PR mode'
        run: |
          if [ "${{ steps.tflint.outputs.exitcode }}" != "0" ]; then
            echo "pr_comment_mode=recreate" >> $GITHUB_ENV 
          else
            echo "pr_comment_mode=delete" >>  $GITHUB_ENV
          fi

      - name: 'Comment PR'
        if: ${{ steps.tflint.outputs.exitcode != '0' || env.pr_comment_mode == 'delete'}}
        uses: thollander/actions-comment-pull-request@v2
        with:
          message: |
            ## :microscope: TFLint validation
            ```shell
            ${{ steps.tflint.outputs.stdout || 'TFLint is running...' }}
            ```
          pr_number: ${{ github.event.pull_request.number }}
          mode: ${{ env.pr_comment_mode }}
          comment_tag: "tflint_comment"
          GITHUB_TOKEN: ${{ secrets.SOME_TOKEN }}

so, if there is any concerns it will be pr_comment_mode=recreate means comment will be added. Everything as expected.
When there is a new commit with changes, but still some concerns from TFLinter, comment will be recreated and Action works as expected.
But if all the TFLint concerns were fixed, then mentioned step sets pr_comment_mode=delete .
In this situation there is a small side effect as delete mode always creates comment. The thing is, there is a situation when it will be two PR comments from this Action, one created when it was TFLint concerns and the new extra short living comment as createComment function will be executed before post step which will remove both comments later.

It will be great if it can be avoided the creation of new intermediate PR comment when mode: delete in the situation I've described.

Maybe, it is kind of "new" mode (cleanup ?) when delete works just as delete, without creation of any comment before the deletion in the end during post step.

@ViacheslavKudinov ViacheslavKudinov changed the title Input create_if_not_exists is ignored on delete mode Input create_if_not_exists is delete mode Mar 17, 2024
@ViacheslavKudinov ViacheslavKudinov changed the title Input create_if_not_exists is delete mode On delete mode new comment always wil be created Mar 17, 2024
@ghost
Copy link

ghost commented Mar 27, 2024

Noticed same behavior as well ..

@ViacheslavKudinov ViacheslavKudinov changed the title On delete mode new comment always wil be created On delete mode new comment always will be created Mar 27, 2024
@mlahargou
Copy link

mlahargou commented Apr 2, 2024

I think we are having the same (or similar) problem. I'd like to just be able to delete comments that have the specified comment_tag. It would be nice if this snippet would just delete the comment if it existed.

         - name: Delete comment if exists
           uses: thollander/actions-comment-pull-request@v2
           with:
              comment_tag: 'some_tag'
              mode: delete

Currently we get an error because either filePath or message is required by the action:

Error: Either "filePath" or "message" should be provided as input

To me, the current delete mode is more like a delete-on-completion mode. But for the sake of backwards compatibility, it might make sense to add a only-delete option which just does:

+++ b/src/main.ts
@@ -157,6 +157,12 @@ async function run() {
             body,
           });
           return;
+        } else if (mode === 'only-delete') {
+          await deleteComment({
+            ...context.repo,
+            comment_id: comment.id,
+          });
+          return;
         } else if (mode === 'delete') {
           core.debug('Registering this comment to be deleted.');

@mlahargou
Copy link

mlahargou commented Apr 2, 2024

Here's a proof of concept with no backwards compatibility issues: iFixit/actions-comment-pull-request@main...iFixit:actions-comment-pull-request:add-only-delete

I can open a PR if you'd like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants