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

Measure quarantine from createdAt #16

Merged
merged 5 commits into from
Oct 4, 2024
Merged

Measure quarantine from createdAt #16

merged 5 commits into from
Oct 4, 2024

Conversation

pbrisbin
Copy link
Member

@pbrisbin pbrisbin commented Oct 4, 2024

Measure quarantine from createdAt, not updateAt

b1e2fbe

Closes #14. See the Issue for explanation of why this is OK and
preferred.

Only comment on PR opened events

1f6df94

Since we're measuring from createdAt, updates don't reset the close.

Update documentation for opened-only usage

5fc9b12

Closes #14. See the Issue for explanation of why this is OK and
preferred.
Since we're measuring from `createdAt`, updates don't reset the close.
@restyled-io restyled-io bot mentioned this pull request Oct 4, 2024
@pbrisbin pbrisbin changed the title pb/no reset Measure quarantine from createdAt Oct 4, 2024
@pbrisbin
Copy link
Member Author

pbrisbin commented Oct 4, 2024

I can't decide if this should be a major version bump or not. It's a very meaningful behavior change. But, if we believe what we're saying about Dependabot's behavior updating PRs, and that using updatedAt was entirely unnecessary, then it could be considered a bug fix.

Making it a major bump is annoying because we have to update all the @v1's to get it. But if we're wrong, and the switch to createdAt actually has impact we're not thinking about (but that we still prefer), signalling that via major is the safer bet.

WDYT @joris974 ?

@pbrisbin pbrisbin marked this pull request as ready for review October 4, 2024 13:05
@pbrisbin pbrisbin requested a review from joris974 October 4, 2024 13:05
@joris974
Copy link
Member

joris974 commented Oct 4, 2024

Based on your arguments and on the fact that this will change the behavior described in the Readme, IMO it will be safer to bump the major.

When configured this way, Mergeabot will run when the PR is opened or updated, so it can let you know that the updates have reset the clock on the quarantine:

@pbrisbin pbrisbin merged commit 140f77e into main Oct 4, 2024
2 checks passed
@pbrisbin pbrisbin deleted the pb/no-reset branch October 4, 2024 16:06
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

Successfully merging this pull request may close these issues.

The quarantine delay resets with each update to a PR
3 participants