paris-branch / dancelor

A chancelor for Scottish country dance musicians
https://dancelor.org
0 stars 0 forks source link

Reworking and functorising filters #221

Closed Niols closed 1 year ago

Niols commented 1 year ago

meant to be squashed

This pull request aims to rework the filters functionality to improve consistency, reduce code duplication, and enhance the interface. The current implementation involves multiple common files for each model, such as *Core, *Lifter, *Filter, *Endpoints, and *Signature, among others. The main challenge in Dancelor models is that we want them consistent between the client and server, but some bits have to be different, in particular the get function. We therefore have *Core that contain the basic definitions (in particular the types) and *Lifter that contain a functor taking other models as parameters and building the whole model. *Signature is then used to enforce that the models on both sides have indeed exactly the same interface.

All is well, but now things like *Filter don't follow this scheme. (Btw, *Filter is a shitty name when there is already *Lifter.) This PR proposes to fix that by moving as many things as possible from *Core and *Filter to *Lifter. This allows bringing code from the client and server sides back to *Lifter whilst improving consistency. Additionally, it will enable the usage of client- or server-specific code within filters, which will be beneficial for future pull requests.

This is however a big refactor that affects pretty much everything model-related. I don't think there is a good way to read this, I mostly made a few changes and then followed the OCaml compiler. However, it is worth building locally and testing. I have done that and things look fine. There are some very tempting follow-up works (such as trying to remove the *Lifted modules altogether, or make *Parameters and SetOrder follow the same treatment) that I will leave for another PR.

This PR is an important milestone towards the use of URIs as part of filters, which I believe will be a very pleasant improvement of Dancelor!

WDYT @R1kM?

Niols commented 1 year ago

I like *Generic or similar names, yeah. We should find something. Logged in https://github.com/paris-branch/dancelor/issues/226.