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

add custom event into the counting of checkout #960

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

Yuyz0112
Copy link
Member

related to #958, #763

I've marked this PR as a draft because I think there are several things we can discuss.

The benefit of counting all events into the checkout calculating

This let the behavior of checkout become more transparent and can use things like custom event and plugin to do fancy features.

But there is also a sub-question of whether we should count all the meta, load, custom, plugin events, or just part of it. For example, the current PR only count custom event because I think the custom events can be controlled by the recorder user.

The downside of this change

  1. It will break some current behavior.
  2. If there is a plugin that can produce a lot of events, it may mass up things.

But we are on the schedule for v2.0, so I think it's good timing if we want to do some changes.

@YunFeng0817
Copy link
Member

IMO, I'd prefer to include all events because it's more consistent with the meaning checkoutEveryNth. I haven't used this config before so I can't provide suggestions from a user's perspective.
BTW, is it possible for us to implement checkoutEveryNms with setInverval?

@Juice10
Copy link
Contributor

Juice10 commented Aug 18, 2022

I also haven't used this in production so I don't have a proper opinion on the matter, but doing it as part of the 2.0 release makes sense to me

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.

3 participants