-
Notifications
You must be signed in to change notification settings - Fork 98
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
Support clear bot history on new build #390
Support clear bot history on new build #390
Conversation
I saw your discussion regarding hiding old bot comments, so you really need to preserve build comments? If not - this would support deleting them. If this doesn't suffice I'll refactor this PR to post a graphql mutation to minimize comments. Let me now... |
I didn't think to just delete the old comments on each run (rather than minimise), but it's not a bad idea to at least test. I do like being able to see the history over time (so prefer minimisation), it can also make it easier to identify flaky tests, and I have some vague concerns about race conditions when builds are simultaneous, but I don't see any reason not to at least test it out. I wonder if we'll hit API request limits in the beginning 😄 @XAMPPRocky did you have any thoughts? It was you who brought up removing/hiding the old test results, so wanted to check in first before deploying anything new. |
It seems fine to me. My one concern about deleting, is that it would make it harder to look at old builds, which sometimes you want to do, but I'm not against deleting either. |
Yeah, same here. But let's try it! If we hate it, we can change it again. I'll get it built and deployed, and will confirm it works on this PR, and assuming it all works, we can roll it out and see how we go! |
Haven't forgotten at this, just been at 100% bandwidth. Do want to get this live so we can take it for a spin, but would like to do it at a time so I can watch in case it goes badly for any reason 😄 |
Deploying now! Let's see how we go! |
Looks like it worked! Let's let it bake for a few days (also would love to get that typo fixed), and if it's going swimmingly, let's merge 👍🏻 |
Build Succeeded 🥳 Build Id: 40fcfcc0-3b81-4fda-a10c-228ddc427fcb To build this version:
|
Let's merge it - it's looking good 👍🏻 |
As per this issue - impl. support to clear bot history on new builds to prevent PR comment bloat.