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

bb acting too fast on no/wontfix #4

Closed
4 tasks done
wooorm opened this issue Apr 23, 2021 · 14 comments
Closed
4 tasks done

bb acting too fast on no/wontfix #4

wooorm opened this issue Apr 23, 2021 · 14 comments

Comments

@wooorm
Copy link
Member

wooorm commented Apr 23, 2021

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and couldn’t find anything (or linked relevant results below)

Affected packages and versions: n/a

Steps to reproduce

Close a PR. Write a comment. BB interferes too fast.

Expected behavior

10m delay?

Actual behavior

syntax-tree/unist-util-visit#23

@ChristianMurphy
Copy link
Member

🤔 Maybe.
An alternative, would it make sense to recommend people post a comment before changing PR status?
BB's speediness/responsiveness is kinda nice.

@wooorm
Copy link
Member Author

wooorm commented Apr 23, 2021

Generally it's fast, but in this case I envision the flow to be 1) label, 2) post human comment. Although i think your flow, the inverse, is working already!
The extra comment is more for maintainers forgetting, rather than for users. Although that could be changed. How do you see this flow?

@wooorm
Copy link
Member Author

wooorm commented Apr 23, 2021

- if-not: recent-comment:team

Recent comment defaults to 1m; so if you comment first and then label within 30 seconds, bb won't comment

@ChristianMurphy
Copy link
Member

I was thinking something along the lines of

                        +--------------+
                        |              |
                        | tag wont-fix |
                        |              |
                        ++------------++
                         |            |
   has no recent comment |            | otherwise
                         |            |
                         v            |
                  +-----------------+ |
                  |                 | |
                  |  BB add comment | |
                  |                 | |
                  +-+--------------++ |
                    |              |  |
maintainer comments |              |  |
                    |              | otherwise
                    v              |  |
       +------------------------+  |  |
       |                        |  |  |
       | BB removes own comment |  |  |
       |                        |  |  |
       +------------+-----------+  |  |
                    |              |  |
                    |              |  |
                    |              |  |
                    v              v  v
               +-------------------------+
               |                         |
               |          done           |
               |                         |
               +-------------------------+

@wooorm
Copy link
Member Author

wooorm commented Apr 24, 2021

That’s almost the case:

beep-boop-beta/tasks.yml

Lines 128 to 133 in d904b1a

- remove-labels:no/{duplicate,external,invalid,question}
- sleep:30s
- if-not: recent-comment:team
then:
- comment:no,no-wontfix
- forget-comments:no
.
Except for the last step, bb removes own comment (as bb’s not yet user comments — and a maintainer can hide it for now manually)

@wooorm
Copy link
Member Author

wooorm commented Apr 24, 2021

So I was thinking of upping it to 10m or so. You’re suggesting to remove the sleep completely?

@ChristianMurphy
Copy link
Member

Except for the last step, bb removes own comment (as bb’s not yet user comments — and a maintainer can hide it for now manually)

BB hiding the comment could be a good alternative to removing it.

So I was thinking of upping it to 10m or so. You’re suggesting to remove the sleep completely?

Yes, that's my current thinking.

@wooorm
Copy link
Member Author

wooorm commented Apr 24, 2021

BB hiding the comment could be a good alternative to removing it.

Hiding it isn‘t hard, but:
a) code to check comments is not written yet
b) running actions on every comment might be too much usage?
c) my flow is to typically label first and then explain in a comment if need be. Are you doing the inverse?

@ChristianMurphy
Copy link
Member

my flow is to typically label first and then explain in a comment if need be. Are you doing the inverse?

I usually do this inverse, add a comment if need be, then tag.

running actions on every comment might be too much usage?

🤷‍♂️ Maybe.
Running for 10 minutes after a tag is added feels like it may be more intensive than a tasks that spins up, sees there's nothing to do, and spins back down. 🤔

@wooorm
Copy link
Member Author

wooorm commented Apr 24, 2021

Running for 10 minutes after a tag is added feels like it may be more intensive than a tasks that spins up, sees there's nothing to do, and spins back down.

While true, I image the ratio of comments to labeled w/ no/wontfix to be 100 to 1.


Maybe this works, allowing both our flows, basically what you envisioned, but still using sleep:

- remove-labels:no/{duplicate,external,invalid,question} 
- comment:no,no-wontfix
- sleep:5m
- if: recent-comment:team 
  then:
    - minimize-comments:no
    - forget-comments:no 

Or even:

- remove-labels:no/{duplicate,external,invalid,question} 
- ifNot: recent-comment:team 
  - comment:no,no-wontfix
  - sleep:5m
  - if: recent-comment:team 
    then:
      - minimize-comments:no
      - forget-comments:no 

@ChristianMurphy
Copy link
Member

While true, I image the ratio of comments to labeled w/ no/wontfix to be 100 to 1

Very possible,
that is potentially balanced out by the fact, that the job currently takes around 12 seconds, and a 10 minute wait would push it over 600 seconds, a ratio of around 50:1.
Running on comments would also allow BB to potentially respond to specific comments, similar to how https://github.com/typescript-bot does on definitely typed PRs.

@wooorm
Copy link
Member Author

wooorm commented Apr 24, 2021

How would you feel about my last suggestion for now, and a new issue to discuss what comment flows would be useful?

@ChristianMurphy
Copy link
Member

I'm open to it.
I wish there was a way to schedule another job, and shut down the process for the intermediate 5 minutes, but sleep may be one of the few options right now.

@wooorm wooorm changed the title bb acting too fast bb acting too fast on no/wontfix May 9, 2021
@github-actions

This comment has been minimized.

wooorm added a commit that referenced this issue May 9, 2021
@wooorm wooorm mentioned this issue May 9, 2021
4 tasks
@wooorm wooorm closed this as completed May 9, 2021
wooorm added a commit that referenced this issue May 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants