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 documentation on how to write a RecordBuilder #739

Open
wants to merge 7 commits into
base: live
Choose a base branch
from

Conversation

diosmosis
Copy link
Member

Description:

As title.

Review

@diosmosis diosmosis added this to the Current sprint milestone Jun 16, 2023
@michalkleiner
Copy link
Contributor

Reading well, thanks! Will read it again but definitely a helpful piece of new docs! :)

@tsteur
Copy link
Member

tsteur commented Dec 21, 2023

Can this be merged @matomo-org/core-reviewers ?

@michalkleiner
Copy link
Contributor

We may need to do a one final sweep but probably very close to being able to merge this.

@AltamashShaikh
Copy link
Contributor

@matomo-org/core-reviewers We should merge this, as this will be helpful for other devs wanting to use Recordbuilders

@michalkleiner
Copy link
Contributor

We probably still need to decide within the core team whether it covers what it needs to cover and maybe remove the unstable information at the top if we deem it stable. But since things run just fine since Matomo 5 around archiving and we haven't seen any significant issues related to archiving it might just be fine as is.

Have you followed the docs and try to create a new record builder, @AltamashShaikh?

@AltamashShaikh
Copy link
Contributor

We probably still need to decide within the core team whether it covers what it needs to cover and maybe remove the unstable information at the top if we deem it stable. But since things run just fine since Matomo 5 around archiving and we haven't seen any significant issues related to archiving it might just be fine as is.

Have you followed the docs and try to create a new record builder, @AltamashShaikh?

@michalkleiner While developing something for AbTesting, I tried looking for a doc but couldn't find anything and I looked different plugins and completed my change, but reading the doc I can confirm the steps mentioned looks correct and would help if someone wants to use RecordBuilder.

@AltamashShaikh
Copy link
Contributor

@diosmosis @michalkleiner Can we please merge this ??

Comment on lines +5 to +10
<div markdown="1" class="alert alert-warning">
**This API is unstable.**

The RecordBuilder API will eventually be public and the only way to define archiving logic, but for now the API is unstable
and subject to change. Please be aware it could potentially change between minor version releases.
</div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still unstable or is this now the default and we can remove this warning?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is up to the core team. feel free to change it yourself when your team decides the answer to that.

@michalkleiner michalkleiner changed the title add document on how to write a RecordBuilder Add documentation on how to write a RecordBuilder Nov 10, 2024
@michalkleiner
Copy link
Contributor

I think this can probably be merged, just posted two questions.

@AltamashShaikh
Copy link
Contributor

I think this can probably be merged, just posted two questions.

@michalkleiner should we merge this now ??

@michalkleiner
Copy link
Contributor

michalkleiner commented Nov 15, 2024

@matomo-org/core-reviewers what do we think? Mainly on the stability aspect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants