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

Repeats the "this pr is too big" message #19

Open
TheTechRobo opened this issue Aug 16, 2020 · 13 comments
Open

Repeats the "this pr is too big" message #19

TheTechRobo opened this issue Aug 16, 2020 · 13 comments
Labels
✨ feature New feature, enhancement or request good first issue Good for newcomers

Comments

@TheTechRobo
Copy link

whenever i push it repeats it. really annoying as i keep getting notifications for nothing.

@JavierCane
Copy link
Member

JavierCane commented Aug 17, 2020

Hi!

As you can see in this example, it publishes the mentioned comment one time for each Workflow execution. That is, it depends on your on clause configuration (example).

Could you please link or attach your yaml configuration and a screenshot of the duplicated message in a PR? (Wipe out sensible information if that's the case)

This could help debugging or reproducing the issue 🙂

Thanks!

@TheTechRobo
Copy link
Author

name: labeler

on: [pull_request]

jobs:
  labeler:
    runs-on: ubuntu-latest
    name: Label the PR size
    steps:
      - uses: codelytv/pr-size-labeler@v1
        with:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          xs_max_size: '10'
          s_max_size: '100'
          m_max_size: '500'
          l_max_size: '1000'
          fail_if_xl: 'false'
          message_if_xl: 'This PR is extremely big! Please, split it 😊'

image

@JavierCane
Copy link
Member

Thanks for the information! 🙌

As we can see in the screenshot, it publishes a comment regarding the PR size being too big for every new commit, that is, on every chance the PR has had to modify its size.

We could have some kind of logic taking into account the previous labels. I mean:

  1. Check if the PR already have the size/xl label and if that's the case:
    1.a. If it's still too big, publish a comment like "This PR is still too big 😬"
    1.b. If it has been reduced from size/xl to any other size, publish a comment like "Congrats! 🎉 You have reduced the PR size successfully!"

What do you think? Would it be better?

@JavierCane JavierCane added good first issue Good for newcomers ✨ feature New feature, enhancement or request labels Aug 19, 2020
@TheTechRobo
Copy link
Author

What do you think? Would it be better?

Yeah! It would also be nice to be able to set e.g. it will check the size every commit, but for commits on the same day it will not say this, i.e. the first commit on august 21 will have the message, but subsequent commits on that day will not show the message, andthe first commit on august 22 will have the message etc to reduce clutter.

@kymikoloco
Copy link

Could it also delete previous xl comments if it's adding another one?

@TheTechRobo
Copy link
Author

Oh yeah, that would be great to reduce clutter.

@TheTechRobo

This comment has been minimized.

@TheTechRobo

This comment has been minimized.

@kymikoloco
Copy link

How's this for a potential solution for deleting? Non-battle tested code (the query seems to work, I didn't run the DELETE) that will remove all previous comments. It should be called before you add a new comment.

# Delete existing messages from the GitHub Actions Bot user that match the $comment
local -r comment_id_list=$(curl -sSL \
    -H "Authorization: token $GITHUB_TOKEN" \
    -H "$GITHUB_API_HEADER" \
    -X GET \
    "$GITHUB_API_URI/repos/$GITHUB_REPOSITORY/issues/$pr_number/comments" \
        | jq -r --arg comment "$comment" '.[] | select(.user.type == "Bot")| select(.user.login == "github-actions[bot]")| select(.body == $comment ) | .id' )

for id in $comment_id_list; do
    curl -sSL \
        -H "Authorization: token $GITHUB_TOKEN" \
        -H "$GITHUB_API_HEADER" \
        -X DELETE \
        "$GITHUB_API_URI/repos/$GITHUB_REPOSITORY/issues/comments/$id"
done

I think this will just have a string of 'comment deleted by' in place of the comments themselves, so I think the 'comment every commit' is still too much.

@TheTechRobo
Copy link
Author

We could in theory impl this with something like this:

https://github.com/aaimio/set-persistent-value

(and its sister repo, https://github.com/aaimio/get-persistent-value)

Basically the persistent value would be a JSON array of PRs that it's already said "This PR is too big!" on, and the script could check the PR against that array

@EngJay
Copy link

EngJay commented Nov 21, 2022

This sticky comment action has a few variations of behavior for replacing or hiding PR comments, so it might be a good reference for an implementation to fix this issue (I haven't had a chance to poke around to see how it works, yet).

@EngJay
Copy link

EngJay commented Nov 21, 2022

Here's the trick - the sticky comment action inserts a comment to use as a means to find the PR comment and replace it.

The h here is the comment text inserted when the comment is made, which is also how the action can post multiple comments for different steps in one job. It has an option to set a string that differentiates the comments.

    const target = repository.pullRequest?.comments?.nodes?.find(
      (node: IssueComment | null | undefined) =>
        node?.author?.login === viewer.login.replace("[bot]", "") &&
        !node?.isMinimized &&
        node?.body?.includes(h)
    )

@TheTechRobo
Copy link
Author

I just thought of something: Maybe you could provide a label name that, if the PR has that label, will prevent the bot from commenting.

That way if there's a known big PR, you can work around this issue by adding that label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature New feature, enhancement or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants